On Thu, Nov 13, 2014 at 1:11 PM, Simon Marlow <marlowsd@gmail.com> wrote:

On 13/11/2014 10:22, Eyal Lotem wrote:

On Thu, Nov 13, 2014 at 10:48 AM, Simon Marlow <marlowsd@gmail.com
<mailto:marlowsd@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@gmail.com <mailto:shumovichy@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@haskell.org <mailto:Libraries@haskell.org>
http://www.haskell.org/__mailman/listinfo/libraries
<http://www.haskell.org/mailman/listinfo/libraries>

_________________________________________________
Libraries mailing list
Libraries@haskell.org <mailto:Libraries@haskell.org>
http://www.haskell.org/__mailman/listinfo/libraries
<http://www.haskell.org/mailman/listinfo/libraries>

_________________________________________________
Libraries mailing list
Libraries@haskell.org <mailto:Libraries@haskell.org>
http://www.haskell.org/__mailman/listinfo/libraries
<http://www.haskell.org/mailman/listinfo/libraries>

--
Eyal

Eyal