Final bikeshedding call: Fixing Control.Exception.bracket
shumovichy at gmail.com
Tue Nov 11 19:58:54 UTC 2014
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 ) 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`
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.
- 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.
On Tue, 2014-11-11 at 10:09 -0800, Merijn Verstraaten wrote:
> In September Eyal Lotem raised the issue of bracket's cleanup handler not being uninterruptible . 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.
> 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.
>  - https://www.haskell.org/pipermail/libraries/2014-September/023675.html
> Libraries mailing list
> Libraries at haskell.org
More information about the Libraries