Maybe potential deadlock in concurrently' ? #189

Hi,

this likely is due to a lack of understanding on my side. Or it's a potential bug.

I'm concerned about the following code in the implementation of concurrently'

    lid <- forkIO $ uninterruptibleMask_ $
      restore (left >>= putMVar done . Right . Left)     --(1)
        `catchAll` (putMVar done . Left)

    rid <- forkIO $ uninterruptibleMask_ $
      restore (right >>= putMVar done . Right . Right)
        `catchAll` (putMVar done . Left)

I'll focus on the computation for lid, but the same holds for rid as well.

In my understanding, the entire expression (left >>= putMVar done . Right . Left), marked (1) above, is run with masking of exceptions restored to the situation when concurrently' was called.

I don't think this is correct.

Let me reformulate the code for lid as follows, which should be eqivalent, just unfolding the line marked (1) above:

    lid <- forkIO $ uninterruptibleMask_ $
      restore (do l <- left
                  putMVar done . Right $ Left l
                  -- MARK
              )
        `catchAll` (putMVar done . Left)

Is it possible, that an exception is caught by the parenthesised expression at MARK, i.e., just after the effect of putMVar manifests? So that restore ( ... ) throws, although done was set? In that case deadlock would occur, because the exception handler would block on putMVar.

Maybe this could be solved by limiting the scope of restore, by changing the code to

    lid <- forkIO $ uninterruptibleMask_ $
      (restore left >>= putMVar done . Right . Left)
        `catchAll` (putMVar done . Left)

and similar for rid?

Or am I mistaken?

Any feedback is welcome!

Cheers
Stefan

P.S.: Some background, I'm using concurrently' via UnliftIO.Async.concurrently. I do occasionally see our testsuite fail with BlockedIndefinitelyOnMVar if the test system is under heavy load, but have no idea which MVar that might be. Unfortunately, I cannot share the codebase, and I've failed to produce a MWE reproducing the issue. It may even be unrelated to my doubts I've expressed above.