Proposal: Use uninterruptibleMask for cleanup actions in Control.Exception

Thu Sep 4 16:03:34 UTC 2014

On Thu, Sep 4, 2014 at 6:16 PM, Eric Mertens <emertens at gmail.com> wrote:

> It's hasn't been noted or I haven't noticed that putMVar is only
> interruptible when the MVar is full.
>
> This means that there isn't a problem in cases like withMVar because the
> MVar is empty due to the establishing takeMVar, and there is no leak.
>
It is plausible that another thread will put into the MVar in the mean time.

> For hClose if you are closing the handle, presumably the contained MVar is
> full and there is no block and therefore no interruption. Only if another
> thread is actively using the handle and you are trying to close the handle
> out from under it is there potential for failure.
>
Only in rare cases is this a bug, I agree. But rare bugs are in many ways
even worse than frequent bugs. They are harder to debug, detect, etc.

> In the exotic cases like this there is already a way to make an
> interruptible operation uninterruptible.
>
> I'm -1 until it becomes clear that it is actually an issue in common code.
>
I think you're missing an important point.

A) Cases that were not interruptible will remain the same.
B) Cases that were interruptible *were bugs* and will be fixed.

You're claiming that B is rare, but I don't think it is a valid argument
against this change, because whether or not you agree B is frequent or not
-- the change only affects B and not A. So the question is whether the
change is desirable in this circumstance.

As an empirical point: My project (buildsome) was very buggy with a lot of
leaks and deadlocks, until I replaced my use of Control.Exception with a
wrapper that properly used uninterruptible-mask.

> On Sep 4, 2014 12:47 AM, "John Lato" <jwlato at gmail.com> wrote:
>
>> Being a primop or not has nothing to do with whether an MVar operation is
>> interruptible.  What matters is whether or not the operation will block.
>>  withMVar (or readMVar in any incarnation) on an empty MVar is
>> interruptible.  If the MVar happened to be full, it's not interruptible.
>>
>> I agree this is a problem.  I don't think the proposed solution is
>> perfect, but I do think it's possibly better than the status quo.  Perhaps
>> the user should be required to use uninterruptibleMask_ on the cleanup
>> action if necessary?  I've long thought that using an interruptible
>> operation in a cleanup handler is a programming error.
>>
>> John L.
>>
>>
>> On Thu, Sep 4, 2014 at 12:29 AM, Eyal Lotem <eyal.lotem at gmail.com> wrote:
>>
>>> In addition to hClose, the majority of cleanup handlers in my programs
>>> turned out to be interruptible. Moreover, whether something is
>>> interruptible is unclear. You can easily add a putStrLn to a cleanup
>>> handler and now it is accidentally interruptible.
>>>
>>> I'd love to see examples of some code where interruptible cleanup
>>> handlers are not a bug.
>>>
>>> Every single one in my programs that I examined was a bug.
>>>
>>> Is withMVar also a primop? Because it's buggy in the same way as
>>> withFile currently is.
>>>
>>> The current situation is that virtually all uses of bracket in the
>>> entire Haskell ecosystem are subtle bugs.
>>> On Sep 4, 2014 3:01 AM, "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
>>>
>>>
>>
>> _______________________________________________
>> Libraries mailing list
>> Libraries at haskell.org
>> http://www.haskell.org/mailman/listinfo/libraries
>>
>>
> _______________________________________________
> Libraries mailing list
> Libraries at haskell.org
> http://www.haskell.org/mailman/listinfo/libraries
>
>

-- 
Eyal
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/libraries/attachments/20140904/98feb3f5/attachment.html>