async
async copied to clipboard
Change semantics of withAsync & double performance of race and concurrently
Hi Simon,
This is an attempt to solve #5 by following your suggested approach of changing the semantics of withAsync
such that if the inner function receives an asynchronous exception it will also be thrown to the forked action. But if it receives a synchronous action the forked action will be killed.
This change in semantics allows for an implementation of race
and concurrently
which only needs to fork a single thread (instead of two) which doubles performance.
I'm pretty sure that concurrently
is correct. I'm less sure about race
because it uses a complex implementation. Although the test cases I added for race
seem to indicate it operates correctly.
Let me know what you think.
This patch was co-written with my colleague @asayers.
I'm beginning to suspect it's impossible to implement a concurrently
(and probably a race
to) which satisfies the spec of:
concurrently left right =
withAsync left $ \a ->
withAsync right $ \b ->
waitBoth a b
but forks only a single thread.
The following test case fails for my current implementation but succeeds for the spec:
concurrently_3 :: Assertion
concurrently_3 = do
rightTidMv <- newEmptyMVar
exMv <- newEmptyMVar
forkIO $ do
threadDelay 1000
rightTid <- takeMVar rightTidMv
throwTo rightTid UserInterrupt
catchIgnore $
concurrently (threadDelay 100000 `catch` putMVar exMv)
(do rightTid <- myThreadId
putMVar rightTidMv rightTid
threadDelay 10000)
ex <- takeMVar exMv
ex @?= UserInterrupt
catchIgnore :: IO a -> IO ()
catchIgnore m = void m `catch` \(e :: SomeException) -> return ()
I will give it another try tomorrow but I suspect I won't succeed :-(
I will give it another try tomorrow but I suspect I won't succeed
The latest commit actually passes the troublesome test-case.
This is a really old PR. Can it be merged?
@3noch perhaps.. I need to give it some serious thought. Is it important for you?
@simonmar I'm not sure. I had a thread that was eating exceptions and took me hours to debug. I haven't tried this patch but it got me thinking.