Re: Final bikeshedding call: Fixing Control.Exception.bracket

On Thu, 2014-11-13 at 21:06 +0000, John Lato wrote:

On 12:31, Thu, Nov 13, 2014 Yuras Shumovich wrote:

TLDR: the proposal is still a bad idea :))))))

IMHO it's better than the current situation.

On Thu, 2014-11-13 at 18:20 +0000, John Lato wrote:

...

hClose is a good example because it's pervasive and currently incorrect. But it's not the only one, and furthermore a lot of broken code is likely in libraries.

hClose is pervasive, but it doesn't mean the bug in it affects a lot of code. Probably 99.(9)% hClose uses are correct. (it may affect more code then I think, but nobody even created a ticket yet!) Do you really think it is common to close handle while using it from other thread?

No, but that's not a general requirement for this problem to manifest.

Sorry, I didn't get it. You mean concurrent access to handle is not a general requirement for the bug in hClose to manifest itself? There are other cases where hClose leaks file descriptor?

Besides, why shouldn't one be able to safely close a handle while another thread may be using it, without using arcane functions?

We *should* be able. So lets fix hClose.

But yes, there is a lot of broken code in libraries. But we should fix bugs instead if hiding them.

This isn't about hiding bugs. It's about changing the semantics of exception handlers so that they are more likely to match what's desired in the general case.

This *is* about hiding bugs. I believe that in most cases async exceptions uncover bugs that are not actually async-only. So using uninterruptibleMask you are hiding real bug. And I already gave an example, I'll reproduce it here: data DB = DB Handle Handle closeDB :: DB -> IO () closeDB (DB h1 h2) = hClose h1 >> hClose h2 Here closeDB is buggy with respect to both sync and async exceptions. Fixing it is trivial: -- here I assume that the bug in hClose is fixed closeDB (DB h1 h2) = hClose h1 `finally` hClose h2 That fixes it w.r.t. sync *and* async exception without any special work for async case. Almost nobody test code for case when file is deleted, so it is unlikely to discover the bug in the first version in case of uninterruptibleMask. So the proposal *hides* the bug, probably for 99% cases, but the bug is here, and it will bit you sooner or later.

The proposal makes it easer to continue ignoring async exceptions. That is why I'm arguing here against it. (Possible breakage if existing code worries me too, but much less)

I think it's a good thing to make it easier to ignore async exceptions.

It is already easy -- just don't use them. Or wrap our main into uninterruptibleMask :) main :: IO () main = uninterruptibleMask_ $ do ....

It is common myth that bracket saves you from async exceptions, but that is simply not true. And the proposal will not make it true. So newcomers learn to ignore async exceptions and continue doing that forever.

Even worse, when newcomer finally find out (most likely itself!!!), that the myth is wrong, he doesn't get help from the community -- no docs, no tutorials, no blogs. Everything he can find is a set of myths.

Or we could make the implementation match the common knowledge. Then it wouldn't be a myth, it would be true.

No, it will *not* be true. Did you read the article I mentioned?

I started learning haskell 8 years ago, and I'm paid for haskell code 3 years already. When do you think I discovered that bracket is not enough to handle async exceptions? Half a year ago(!)

I'll post the link again: http://haskell.1045720.n5.nabble.com/Control-Exception-bracket-is-broken-td5... Note that nobody answered the question about hClose. Nobody explained how to use interruptible exceptions in cleanup. Very few people actually even care to replay. Because everybody learned to ignore async exceptions.

Did you read this: http://www.well-typed.com/blog/97/ ? I hope you did. Because it is probably the *only* deep discussion of *some* of the related issues. Unfortunately it appears after too late for me, so I spent a lot of days discovering everything myself.

Wouldn't it be better if stuff like that just worked out of the box? People could still spend days learning it, but they wouldn't be bitten by these hard-to-identify, poorly-understood bugs.

I even wrote my own library for exception handling: https://github.com/Yuras/io-region/ I'm still not sure it is good. It even can be buggy. But I considered a lot of design decisions, including unintrruptibleMask, and found then unsatisfactory

.

I personally find it easer to write exception safe code with io-region, but you should understand async exceptions anyway. There is no easy way unfortunately.

That doesn't mean we should make it harder to do the right thing.

The proposal doesn't make it significantly easer. It just hides bugs. It is a myth that handling async exceptions in cleanup is much harder then handling sync exceptions. Async exceptions are hard to deal with because they can be raised at any point (even between points :) ). But in cleanup action async exceptions are masked, and can be raised only in well known points, just like regular sync exceptions.