`takeResource` is not async exception safe

[parsonsmatt]

Current implementation:

takeResource :: Pool a -> IO (a, LocalPool a)
takeResource pool = mask_ $ do
  lp <- getLocalPool (poolNumStripes $ poolConfig pool) (localPools pool)
  stripe <- takeMVar (stripeVar lp)
  if available stripe == 0
    then do
      q <- newEmptyMVar
      putMVar (stripeVar lp) $! stripe { queueR = Queue q (queueR stripe) }
      waitForResource (stripeVar lp) q >>= \case
        Just a  -> pure (a, lp)
        Nothing -> do
          a <- createResource (poolConfig pool) `onException` restoreSize (stripeVar lp)
          pure (a, lp)
    else takeAvailableResource pool lp stripe

This code does a mask_, which means that async exceptions will only be delivered on a blocking call for the duration of mask_.

So, if I do:

do
  tid <- myThreadId
  forkIO $ threadDelay 100 $ killThread tid
  (a, localPool) <- takeResource pool

This will send the async exception to takeResource. takeResource will complete, and then the thread blows up with an async exception. This allows for resources to leak, and is why #10 can't simply be implemented as timeout $ takeResource pool.

[parsonsmatt]

Hmm, that's not why #10 can't be implemented like that. I copied the tests into the #21 branch and implemented there, and it works fine.

[arybczak]

takeResource is async exception safe, because

This will send the async exception to takeResource. takeResource will complete

What happens after that is the code you wrote and it's your code that's not async exception safe.

Also, in your example takeResource will complete only if takeMVar in waitForResource doesn't block (or something in createResource doesn't block).

So timeout $ takeResource pool is a correct implementation for #10.