Proposal: Use uninterruptibleMask for cleanup actions in Control.Exception

[Merijn Verstraaten]

Handles use MVar’s internally, therefore any operation on a Handle can potentially block, making the operation interruptible.

FWIW, I’m +1 on this. It’s been a pain to get this correct in my code.

Cheers,
Merijn

On 03 Sep 2014, at 17:01 , Gregory Collins <greg at gregorycollins.net> wrote:
> Unless I'm mistaken, here the "mask" call inside bracket already makes sure you don't receive asynchronous exceptions unless you call a function that is interruptible (i.e. goes back into the runtime system). The hClose example you give doesn't fall in this category, as something inside the RTS needs to call "allowInterrupt" (or otherwise unmask exceptions) in order for async exceptions to be delivered. The "readMVar" example you give *does* have this issue (because putMVar does an implicit allowInterrupt) but in recent GHC readMVar has been redefined as a primop.
> 
> The danger of deadlock is *not* minimal here, doing what you suggest will transform many valid programs (i.e. if you block on a "takeMVar" in the cleanup action) into ones that have unkillable orphan threads.
> 
> G
> 
> 
> On Wed, Sep 3, 2014 at 1:56 PM, Eyal Lotem <eyal.lotem at gmail.com> wrote:
> I'd like to propose a change in the behavior of Control.Exception to help guarantee cleanups are not accidentally lost.
> 
> For example, bracket is defined as:
> bracket before after thing =
>   mask $ \restore -> do
>     a <- before
>     r <- restore (thing a) `onException` after a
>     _ <- after a
>     return r
> This definition has a serious problem: "after a" (in either the exception handling case, or the ordinary case) can include interruptible actions which abort the cleanup.
> 
> This means bracket does not in fact guarantee the cleanup occurs.
> 
> For example:
> 
> readMVar = bracket takeMVar putMVar -- If async exception occurs during putMVar, MVar is broken!
> 
> withFile .. = bracket (openFile ..) hClose -- Async exception during hClose leaks the file handle!
> 
> Interruptible actions during "before" are fine (as long as "before" handles them properly).  Interruptible actions during "after" are virtually always a bug -- at best leaking a resource, and at worst breaking the program's invariants.
> 
> I propose changing all the cleanup handlers to run under uninterruptibleMask, specifically:
> 
> bracket, bracketOnError, bracket_, catch, catchJust, finally, handle, handleJust, onException
> 
> should all simply wrap their exception/cancellation handler with uninterruptibleMask.
> 
> The danger of a deadlock is minimal when compared with the virtually guaranteed buggy incorrect handling of async exceptions during cleanup.
> 
> -- 
> Eyal
> 
> _______________________________________________
> Libraries mailing list
> Libraries at haskell.org
> http://www.haskell.org/mailman/listinfo/libraries
> 
> 
> 
> 
> -- 
> Gregory Collins <greg at gregorycollins.net>
> _______________________________________________
> Libraries mailing list
> Libraries at haskell.org
> http://www.haskell.org/mailman/listinfo/libraries

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://www.haskell.org/pipermail/libraries/attachments/20140903/d3c91b33/attachment.sig>