fpco / unliftio

The MonadUnliftIO typeclass for unlifting monads to IO
Other
151 stars 51 forks source link

Should timeout/race forcibly unmask async exceptions? #74

Open brandon-leapyear opened 3 years ago

brandon-leapyear commented 3 years ago

timeout and race use async exceptions to cancel a timed-out action. IMO, the fact that these use async exceptions are an implementation detail, so I would think the user shouldn't have to care about the fact that these use async exceptions.

For example, if someone were to do

action `finally` timeout (60 * 1000000) cleanup

I think the user would be surprised that it doesn't timeout.

snoyberg commented 3 years ago

It's a fair point, but I'd be concerned about differences in behavior versus upstream functions. We already have that with bracket, and I'm already concerned that's too much of a distinction. I'm not saying no, because I think you have a valid point, just some reservation.

To throw it into the mix, another interesting possibility would be throwing an exception when timeout or race are called with async exceptions are masked.

But more directly on the merits: there may be arguably-valid reasons to do something like that, such as ensuring that some work happens before terminating the thread. For example:

mask $ \restore -> race (restore oneThing) (bracket_ (putStrLn "starting second thing") (putStrLn "stopping second thing") (restore secondThing))

I'm not arguing this is great code. Virtually any code that uses mask is bad code (including my own!). But it's worth considering.

brandon-leapyear commented 3 years ago

:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:


Yeah, I haven't thought about this any more than surface-level, and I'm pretty sure the code I'm thinking of can be written in a better way anyway. But I'll leave this ticket up just for future reference, in case people want to add on to the conversation