simonmar / async

Run IO operations asynchronously and wait for their results
BSD 3-Clause "New" or "Revised" License
322 stars 65 forks source link

race somehow changes async exception semantics? #63

Closed osa1 closed 6 years ago

osa1 commented 7 years ago

Summary: race a waitForever works differently than just a.

I have this program:

{-# LANGUAGE ScopedTypeVariables #-}

import Control.Concurrent
import Control.Concurrent.Async
import Control.Concurrent.Chan
import Control.Concurrent.MVar
import Control.Exception
import Data.Functor
import System.Mem

doWork
    :: MVar () -- ^ Stop signal
    -> IO (Either () ())
doWork stop_ref =
    race (readMVar stop_ref `catch` \(e :: SomeException) -> print e)
         (threadDelay maxBound)

main :: IO ()
main = do
    var <- newEmptyMVar
    thr <- async (doWork var)
    performMajorGC
    wait thr >>= print

This program blocks forever. However, if I change doWork to this:

doWork stop_ref =
    Right <$> (readMVar stop_ref `catch` \(e :: SomeException) -> print e)

It prints exception messages as expected.

Just as an experiment, I tried using this race function:

race' left right =
  withAsync left $ \a ->
  withAsync right $ \b ->
  waitEither a b

but it still didn't print the expected messages.

I would expect this to hold: forall a . race a waitForever == Left <$> a. I think we should at least understand and explain this behavior in the documentation.

ChristopherKing42 commented 7 years ago

waitEither says that if the first Async to finish raises an exceptions, it should re-raise that exception. So there is a bug somewhere (maybe in the garbage collection code outside this library).

simonmar commented 6 years ago

This is perhaps counterintuitive, but it's working as expected. The GC can't detect a deadlock because the main thread has a reference to both child threads, and one of the child threads is in threadDelay. The GC can't prove that threadDelay will never return, or that the main thread won't decide to cancel the readMVar thread at some point, so there's no verifiable deadlock here.

The thing to bear in mind is that BlockedIndefinitelyOnMVar is a debugging feature. We can't give a precise description of its semantics because it depends on very operational properties (like reachability) that can't be precisely described at the language level. So when BlockedIndefinitely exceptions are involved, some properties that we might expect to hold (like race a waitForever == Left <$> a) don't.

Is this bad? Arguably yes, but then having the runtime detect deadlocks is also tremendously useful. cc @ndmitchell

alaendle commented 1 year ago

@simonmar - I just try to fully internalize the behavior - so is this also the reason why this code needs 100 seconds to complete?

main :: IO ()
main = do
    a <- Async.async $ do
       queue <- STM.newTBMQueueIO 100
       STM.atomically (STM.readTBMQueue queue)

    void $ Async.race (Async.wait a) (threadDelay 100_000_000)

It's really counterintuitive since a single void $ Async.wait a immediately shows thread blocked indefinitely in an STM transaction- and also void $ Async.race (Async.wait a) (Async.wait a).

So the intuition would tell me that if Async.race (Async.wait a) (Async.wait a) immediately returns - this should also be true for Async.race (Async.wait a) (everythingElse) 😄

I was also able to create way more sophisticated real-world examples involving different threads for reading/writing to the TBM queue and also use of MVar's - but I think they boil down to basically the few lines shown above. And to be honest I haven't fully understood the behavior also after studying concurrently'.

Thanks in advance anyone who can shed some light on this.

simonmar commented 1 year ago

@alaendle Think of it this way: Async.wait a will block forever if (1) a is blocked forever and (2) nothing can cancel the current thread. Inside a race, (2) becomes false because the race can cancel the Async.wait.

Using weak pointers to ThreadIds might be able to fix this. Maybe worth looking into.