Final bikeshedding call: Fixing Control.Exception.bracket
Simon Marlow
marlowsd at gmail.com
Thu Nov 13 11:11:31 UTC 2014
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 ...".
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
More information about the Libraries
mailing list