Final bikeshedding call: Fixing Control.Exception.bracket
Eyal Lotem
eyal.lotem at gmail.com
Thu Nov 13 13:43:55 UTC 2014
On Thu, Nov 13, 2014 at 1:11 PM, Simon Marlow <marlowsd at gmail.com> wrote:
> On 13/11/2014 10:22, Eyal Lotem wrote:
>
>>
>> On Thu, Nov 13, 2014 at 10:48 AM, Simon Marlow <marlowsd at gmail.com
>> <mailto:marlowsd at gmail.com>> wrote:
>>
>> I think the bigger objection to using uninterruptibleMask for the
>> allocation phase of bracket is that it breaks this:
>>
>> withMVar m io = bracket (takeMVar m) (putMVar m) io
>>
>> Now withMVar will be uninterruptible while it is blocked, which will
>> make a lot of common idioms unresponsive to async exceptions. This
>> was the whole motivation behind the idea of interruptible operations.
>>
>>
>> But do you really prefer the alternative to this unresponsiveness?
>>
>
> To clarify I was responding here to the suggestion that the "allocation
> phase" (the first argument to bracket) should use uninterruptibleMask. The
> email I replied to quoted below begins "Allocation should not use
> uninterruptibleMask ...".
>
Oh, I see. I think there's a consensus that allocation cancellation is a
very useful thing and must remain supported. i.e: the uninterruptibleMask
only applies to the cleanup.
>
> Cheers,
> Simon
>
>
> The alternative here is responsiveness, but it surely breaking the
>> invariants of this MVar (the number of takes/puts will no longer match).
>>
>> I think it is far more important to maintain these kinds of invariants
>> than to be responsive to async exceptions at any price.
>>
>> Also, if this is not responsive -- it means that someone is holding up
>> the MVar, and responsiveness can still be attained by making sure that
>> the holder stops hogging the mvar. That way you can maintain your
>> invariants and be responsive, and this is the kind of approach I've used
>> in my programs after using uninterruptibleMask for cleanups.
>>
>>
>> Cheers,
>> Simon
>>
>>
>> On 11/11/2014 20:17, Merijn Verstraaten wrote:
>>
>> Allocation should not use uninterruptibleMask as it is possible
>> to handle async exceptions during allocation by nesting
>> bracketOnError
>>
>> Example:
>> someFun mvar1 mvar2 = do
>> (val1, val2) <- bracketOnError
>> (takeMVar mvar1)
>> (putMVar mvar1)
>> (\x -> takeMVar mvar2 >>= \y -> return (x, y)))
>>
>> This can be made nicer using the Cont monad to hide the marching
>> to the left. The same cannot be done for cleanup, as there's no
>> sane thing as "half a cleanup".
>>
>> I disagree that it should be left to the author of allocation
>> operation to ensure uninterruptibility as it is impossible to
>> know whether a given IO blocks internally and thus should be
>> masked without inspecting the *entire* code path potentially
>> called by the cleanup handler.
>>
>> Both Eyal and me have had trouble with this where we had to
>> entire half of base and part of the runtime, to figure out
>> whether our code was async exception safe. Auditing half the
>> ecosystem to be able to write a safe cleanup handler is *NOT* a
>> viable option.
>>
>> Cheers,
>> Merijn
>>
>> On 11 Nov 2014, at 11:58, Yuras Shumovich
>> <shumovichy at gmail.com <mailto:shumovichy at gmail.com>> wrote:
>>
>> Hello,
>>
>> Should we use `uninterrubtibleMask` for allocating action too?
>>
>>
>> I'm not sure my voice will be counted, but anyway,
>> I'm strong -1 because it fixes wrong issue.
>>
>> `hClose` is interruptible, but it closes the handle in any
>> case. I'm
>> pretty sure. I ask that question (see
>> http://haskell.1045720.n5.__nabble.com/Control-Exception-__
>> bracket-is-broken-td5752251.__html
>>
>> <http://haskell.1045720.n5.nabble.com/Control-Exception-
>> bracket-is-broken-td5752251.html>
>> ) but didn't get any answer, so I read code and made
>> experiments. IIRC `hClose` wraps internal interruptible
>> action into `try` and handles everything correctly.
>>
>> I argue that cleanup action can be interruptible, but should
>> ensure
>> cleanup is done. As the last resort, it should use
>> `uninterrubtibleMask`
>> internally.
>>
>> Other issue is that a lot of allocating action are broken
>> because they
>> perform interruptible actions after allocating resource
>> without handling
>> async exceptions. So my point is that masking async
>> exceptions solves
>> only one half of the issue while masking the other.
>>
>> Handling async exceptions is hard, and we can't make is easy
>> using
>> `uninterrubtibleMask`. Instead we should educate ourselves
>> to do it
>> correctly from the very beginning. There is only one
>> alternative --
>> remove async exceptions from haskell.
>>
>> To summarize,
>> - allocating action should either allocate resource or throw
>> exception;
>> it is a bug to allocate resource *and* throw exception
>> - cleanup action should release resource even if it throws
>> an exception
>> Developer should ensure both properties holds.
>>
>> Sorry my poor English.
>>
>> Thanks,
>> Yuras
>>
>> On Tue, 2014-11-11 at 10:09 -0800, Merijn Verstraaten wrote:
>>
>> Ola!
>>
>> In September Eyal Lotem raised the issue of bracket's
>> cleanup handler not being uninterruptible [1]. This is a
>> final bikeshedding email before I submit a patch.
>>
>> The problem, summarised:
>> Blocking cleanup actions can be interrupted, causing
>> cleanup not to happen and potentially leaking resources.
>>
>> Main objection to making the cleanup handler
>> uninterruptible:
>> Could cause deadlock if the code relies on async
>> exceptions to interrupt a blocked thread.
>>
>> I count only two objections in the previous thread, 1 on
>> the grounds that "deadlocks are NOT unlikely" and 1 that
>> is conditioned on "I don't believe this is a problem".
>>
>> The rest seems either +1, or at least agrees that the
>> status quo is *worse* than the proposed solution.
>>
>> My counter to these objections is:
>> 1) No one has yet shown me any code that relies on the
>> cleanup handler being interruptible
>>
>> 2) There are plenty of examples of current code being
>> broken, for example every single 'bracket' using file
>> handles is broken due to handle operations using a
>> potentially blocking MVar operation internally,
>> potentially leaking file descriptors/handles.
>>
>> 3) Even GHC-HQ can't use bracket correctly (see Simon's
>> emails)
>>
>> Potential solution #1:
>> Leave bracket as-is, add bracketUninterruptible with an
>> uninterruptible cleanup handler.
>>
>> Potential solution #2:
>> Change bracket to use uninterruptible cleanup handler,
>> add bracketInterruptible for interruptible cleanups.
>>
>> Trade-offs:
>> Solution 1 won't change the semantics of any existing
>> code, however this also means that any currently broken
>> uses of bracket will remain broken, possibly indefinitely.
>>
>> Solution 2 will change the semantics of bracket, which
>> means any currently broken uses of bracket will be
>> fixed, at the cost of creating potential deadlocks in
>> code that relies on the interruptibility of cleanup.
>>
>> I will argue that solution #2 is preferable, since I
>> have yet to see any code that uses the interruptibility
>> of the cleanup handler. Whereas there's many broken
>> assumption assuming the cleanup handler is not
>> interruptible.
>>
>> Secondly, it is easier to detect deadlocks caused by
>> this problem than it is to detect resource leaks which
>> only happen in unlucky timings of async exceptions.
>> Especially since any deadlock caused by the change can
>> be fixed by replacing bracket with bracketInterruptible.
>>
>> [1] -
>> https://www.haskell.org/__pipermail/libraries/2014-__
>> September/023675.html
>> <https://www.haskell.org/pipermail/libraries/2014-
>> September/023675.html>
>>
>> Cheers,
>> Merijn
>> _________________________________________________
>> Libraries mailing list
>> Libraries at haskell.org <mailto:Libraries at haskell.org>
>> http://www.haskell.org/__mailman/listinfo/libraries
>> <http://www.haskell.org/mailman/listinfo/libraries>
>>
>>
>>
>>
>> _________________________________________________
>> Libraries mailing list
>> Libraries at haskell.org <mailto:Libraries at haskell.org>
>> http://www.haskell.org/__mailman/listinfo/libraries
>> <http://www.haskell.org/mailman/listinfo/libraries>
>>
>> _________________________________________________
>> Libraries mailing list
>> Libraries at haskell.org <mailto:Libraries at haskell.org>
>> http://www.haskell.org/__mailman/listinfo/libraries
>> <http://www.haskell.org/mailman/listinfo/libraries>
>>
>>
>>
>>
>> --
>> Eyal
>>
>
--
Eyal
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.haskell.org/pipermail/libraries/attachments/20141113/e803ae98/attachment-0001.html>
More information about the Libraries
mailing list