Add `Control.Concurrent.Chan.tryReadChan`

[konsumlamm]

There is currently no way to read from a Chan without blocking when the channel is empty. I propose to add a new function

-- | Read the next value from the 'Chan' if one is available. Returns 'Nothing' if the channel is empty.
tryReadChan :: Chan a -> IO (Maybe a)

This is useful if the receiver can do something else when no elements are available in a channel or doesn't want to potentially wait forever.

However, there's a caveat: tryReadChan might still block at taking the read MVar if there are several threads trying to read from the same channel. This can be avoided by using dupChan.

The implementation should be straightforward, like for readChan but using tryReadMVar instead of readMVar.

Prior Art

• stm has tryReadTChan
• BoundedChan has tryReadChan (with the same caveat)
• unagi-chan has tryReadChan (with a slightly different signature)

[mixphix]

Seems like a good design pattern to me. I'm ok with the name you've picked, with try- instead of -Maybe, because it is still fundamentally an IO action.

[konsumlamm]

The try prefix is also consistent with Control.Concurrent.MVar, which has tryTakeMVar, tryPutMVar and tryReadMVar.

[parsonsmatt]

I may be misremembering, so feel free to double check me here. One aspect of the other try functions is that they don't block, and so won't be interrupted by an async exception while in MaskedInterruptible state. However, the straightforward implementation of just replacing readMVar with tryReadMVar won't accomplish this, as modifyMVar blocks to wait on the MVar for the read end. I think the try nomenclature may be misleading in this case.

That BoundedChan introduces the caveat is (IMO) not a sufficient justification to bring it into base. I'd personally rather see the "try means non-blocking (aka async exceptions won't be raised while masked)" idiom encouraged in that package, too. If others agree that "try means non-blocking" in this context, then readChanMaybe is a more suitable name.

I think you can get a non-blocking tryPeekChan which doesn't remove an element from the Chan - tryReadMVar read >>= \mchan -> maybe (pure Nothing) tryReadMVar mchan.

The straightforward implementation of tryReadMVar will cause this code to block:

main :: IO ()
main = do
    chan <- newChan
    forkIO $ do
        a <- readChan chan
        -- now we've got a reader blocking on the chan, waiting an input
        print ("From forked thread", a)

    threadDelay 10_000

    ma <- tryReadChan chan -- chan's read end is blocked
    case ma of
        Nothing -> writeChan chan ()
        Just a -> print ("From main thread, a)

I think a function that promises to "return immediately with a Nothing if the channel is empty," but actually blocks if any other readers are blocked is a pretty dangerous footgun. I feel like the usual pattern for writing code on these is to withAsync (forever $ readChan chan >>= process) \chanWorkerThread -> ... or similar - which means that a Chan is usually going to be in a state with a blocked reader thread, intending to signal "stop" with an async exception.

If the implementation performs a dupChan immediately beforehand - well, dupChan does a readMVar writeVar which blocks, so you'd need to tryReadMVar writeVar in case the writeChan is empty. Additionally, dupChan won't cause it to read the element and remove it from other copies of that Chan, which makes it not really a proper tryReadChan but instead a tryPeekChan.

[konsumlamm]

One aspect of the other try functions is that they don't block, and so won't be interrupted by an async exception while in MaskedInterruptible state. However, the straightforward implementation of just replacing readMVar with tryReadMVar won't accomplish this, as modifyMVar blocks to wait on the MVar for the read end.

Yes, that's the caveat I mentioned. I think to solve this problem, we'd need something like a condition variable, to release the read MVar when waiting for an element and reacquire it when woken, but I'm not aware of a Haskell implementation for this.

That BoundedChan introduces the caveat is (IMO) not a sufficient justification to bring it into base. I'd personally rather see the "try means non-blocking (aka async exceptions won't be raised while masked)" idiom encouraged in that package, too. If others agree that "try means non-blocking" in this context, then readChanMaybe is a more suitable name.

Nothing about try inherently says "non-blocking" (although that is the behaviour for the try* MVar operations). If noone else is reading from the same channel (which is the case when each consumer uses dupChan or there's only one consumer), it is in fact non-blocking, so I think it would still make sense. I'd also be ok with readChanMaybe, but I'd prefer tryReadChan.

I think you can get a non-blocking tryPeekChan which doesn't remove an element from the Chan - tryReadMVar read >>= \mchan -> maybe (pure Nothing) tryReadMVar mchan.

This would also return Nothing if another thread is currently reading from the same channel, even if it isn't actually empty.

The straightforward implementation of tryReadMVar will cause this code to block:

Which can be fixed by calling dupChan at the beginning and then using the duplicated channel in the main (or the forked) thread.

If the implementation performs a dupChan immediately beforehand - well, dupChan does a readMVar writeVar which blocks, so you'd need to tryReadMVar writeVar in case the writeChan is empty. Additionally, dupChan won't cause it to read the element and remove it from other copies of that Chan, which makes it not really a proper tryReadChan but instead a tryPeekChan.

I don't mean that the implementation should use dupChan, but rather the user when there are multiple threads reading from a channel.

[mixphix]

The straightforward implementation of tryReadMVar will cause this code to block:

Which can be fixed by calling dupChan at the beginning and then using the duplicated channel in the main (or the forked) thread.

That was my impression from reading the documentation: dupChan is meant to copy a write-only broadcast channel into one or more read-only channels, that would then each be free to use readChan on their own channel without blocking the write channel, all while receiving the same broadcast.

Provided this is the case, tryReadChan strikes me as useful to replace persnickety exception-handling code with less finicky Maybe case analysis.

[konsumlamm]

Ideally, readChan would not hold the read MVar while waiting for an element to read, but I don't think that's currently possible (it might be possible with something like https://gitlab.haskell.org/ghc/ghc/-/issues/23875 though).

[Bodigrim]

@konsumlamm could you please prepare an MR so that we can discuss a specific implementation?

[konsumlamm]

MR: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11267

[Bodigrim]

I must admit that I never used Control.Concurrent.Chan and thus do not hold an informed opinion on the topic.

@parsonsmatt @bgamari @simonmar @nikita-volkov could you possibly opine on this?

[nikita-volkov]

Thanks for pinging me. However I'm not a user of this module either, so I'm lacking an informed opinion as well.

[parsonsmatt]

-1 with the tryReadChan name given the try connotation of "non-blocking" in other MVar-related code

+1 to readChanMaybe

[Bodigrim]

@tomjaguarpaw @angerman @tomjaguarpaw @hasufell any more opinions on this? I'm honestly completely illiterate in the area.

@parsonsmatt am I right that you are happy with the suggested implementation modulo tryReadChan vs. readChanMaybe?

[tomjaguarpaw]

I'm very cautious about voting in favour of a new concurrency primitive; I don't feel I have the expertise necessary to determine whether it's implemented safely, for example, avoids deadlocks and race conditions. If unagi-chan already supports this operation, can those who want this feature not just use unagi-chan instead? It describes itself as "a general-purpose near drop-in replacement for Chan".

If we really think this needs to be in base can we get an independent assessment from a Concurrent Haskell expert, such as @simonmar?

[parsonsmatt]

The implementation details look fine to me, provided the name is changed

[Bodigrim]

@konsumlamm how would you like to proceed here? Are you prepared to change the name as suggested by @parsonsmatt? Is there a way to alleviate concerns raised by @tomjaguarpaw?

[konsumlamm]

I'd be fine with changing the name to readChanMaybe. How do others feel about the name? I'd also appreciate feedback from someone with more concurrency experience (eventhough I can't imagine anything going erong here, the implementation is almost the same as for readChan).

Another option would be to recommend using other channel inplementations instead (TChan, unagi-chan), although STM also has its tradeoffs and unagi-chan doesn't seem to be actively developed (the last Hackage upload was in 2021).

cc @jberryman

[mixphix]

It seems none of the tagged users in this thread have any valuable input. For that reason I wonder if this function is a necessary addition to base, and if it would be more suited to a principled library tailored specifically to using Chan values safely. In fact I wonder if Chan as an abstraction is a vital part of the GHC interface, or if perhaps that entire module may have a better home elsewhere.

[bgamari]

In fact I wonder if Chan as an abstraction is a vital part of the GHC interface, or if perhaps that entire module may have a better home elsewhere.

Frankly I generally recommend that people avoid using Chan at all. If no non-boot dependencies are acceptable then TChan is a strict improvement over Chan. Otherwise an external package like unagi-chan may be preferable for performance reasons.

[Bodigrim]

Another option would be to recommend using other channel inplementations instead (TChan, unagi-chan), although STM also has its tradeoffs and unagi-chan doesn't seem to be actively developed (the last Hackage upload was in 2021).

@konsumlamm I think recommending TChan over Chan more strongly is a good approach. Control.Concurrent.Chan already says that "The stm (software transactional memory) library has a more robust implementation of channels called TChans", shall we make the warning language stronger?

[Bodigrim]

@konsumlamm shall we bring this to a conclusion one way or another? It seems there are two options:

• Go ahead with a vote on readChanMaybe.
• Update Control.Concurrent.Chan to recommend TChan more persistently. Documentation changes do not require a vote.

[Bodigrim]

@konsumlamm let's make some substantial progress here, otherwise I'll close it as abandoned in two weeks.

[konsumlamm]

IMO the wording recommending TChan is fine, although it would make sense to link it in the documentation. I'm fine with dropping this proposal and only updating the documentation. What do you think about recommending/linking unagi-chan? It seems to be a strictly better implementation, however it hasn't been updated in more than 2 years and is a 3rd party library.

[Bodigrim]

In general base documentation is not shy of recommending third-party libraries when appropriate. One can recommend both TChan and unagi-chan.

[Bodigrim]

I'm fine with dropping this proposal and only updating the documentation.

Let me close the issue then.