Final bikeshedding call: Fixing Control.Exception.bracket

[Yuras Shumovich]

On Fri, 2014-11-14 at 00:33 +0200, Eyal Lotem wrote:
> On Fri, Nov 14, 2014 at 12:24 AM, Yuras Shumovich <shumovichy at gmail.com>
> wrote:
> > That fixes it w.r.t. sync *and* async exception without any special work
> > for async case.
> >
> 
> NO: This new code is still broken. Async exception in close of h1 will just
> "jump" to block at h2, and yield the full side effect of closing h2 while
> skipping some of the side effect of closing h1.

Yes, but that is good default, I already explained why. If you care
about about side effect in case of async exception, when use
uninterruptibleMask explicitly, and explain in comments why you need
that.

> 
> All my code used these kind of idioms for sync-exception-safety but was
> still ridden with bugs w.r.t async exceptions.

Several options:
a) your code is buggy, fix it
b) you rely on buggy code, pester it's author to fix it (and temporary
use uninterruptibleMask)
c) you really need uninterruptibleMask here, go ahead and use it

> 
> The kill&wait for process are another example, but I have multiple examples
> -- all of which become more reasonable when the cancellation is
> uninterruptible.

Then just use uninterruptibleMask if it is reasonable in your case.

> 
> >
> > Almost nobody test code for case when file is deleted, so it is unlikely
> > to discover the bug in the first version in case of uninterruptibleMask.
> > So the proposal *hides* the bug, probably for 99% cases, but the bug is
> > here, and it will bit you sooner or later.
> >
> 
> Are you talking about Windows or POSIX? With POSIX, file deletion has
> nothing to do with hClose.

Ok, Almost nobody test code for case when <insert a case when your
cleanup action throws sync exception>

> 
> 
> >
> >
> > > The proposal makes it easer to continue ignoring async exceptions. That
> > > is why I'm arguing here against it. (Possible breakage if existing code
> > > worries me too, but much less)
> > >
> > >  I think it's a good thing to make it easier to ignore async exceptions.
> >
> > It is already easy -- just don't use them.
> >
> 
> Whenever you use the async library, for example, you use async exceptions.
> And then all your bracket invariants are broken *by-default*.

Don't use async then.

> 
> 
> > Or wrap our main into uninterruptibleMask :)
> >
> > main :: IO ()
> > main = uninterruptibleMask_ $ do
> >   ....
> >
> 
> But cancelling/killing non-cleanups is not problematic in general.

Did you read "Dealing with Asynchronous Exceptions during Resource
Acquisition" article?

> >
> > It is a myth that handling async exceptions in cleanup is much harder
> > then handling sync exceptions. Async exceptions are hard to deal with
> > because they can be raised at any point (even between points :) ). But
> > in cleanup action async exceptions are masked, and can be raised only in
> > well known points, just like regular sync exceptions.
> >
> 
> Sync exceptions are under your control. You can make sure the preconditions
> of the operation are met so that they are not raised. If they are raised,
> they are related to the operation at hand so there may be something that
> you can do.  Async exceptions are fundamentally different, so please stop
> mishmashing these two dissimilar things together.
> 

So you want to ban sync exceptions in cleanup actions?