Receiver is not multi-consumer

[link2xt]

README describes async-broadcast as an MPMC channel similar to async-channel:

async-broadcast/README.md

Line 65 in 74d5889

This crate is similar to [`async-channel`] in that they both provide an MPMC channel but the main

However, in case of async-broadcast Receiver.recv(), Receiver.recv_direct() and Receiver.try_recv() take &mut self instead of &self. This means it is impossible to have multiple threads taking messages from a single Receiver side, e.g. to distribute some kind of work between threads. Naive solution of wrapping Receiver into async mutex does not work because any thread holding the mutex while calling recv() will also block any thread calling try_recv() so try_recv() will not return immediately as it should. Having multiple Receivers also does not make the channel multi-consumer, in a way creating a receiver creates a new "channel" with its own buffer that is subscribed to the same broadcast sender.

Is there technical limitation that prevents making receiving calls take non-mut reference &self? If it is not possible to change e.g. for performance reasons, I think at least the documentation should be corrected to describe why async-broadcast differs from async-channel in this aspect.

[zeenix]

This means it is impossible to have multiple threads taking messages from a single Receiver side,

No, it does not. You can clone the receiver as many times as you need and give it out to threads/tasks. Unlike, async-channel, each receiver gets a clone of each message sent to the channel. I'm using async-broadcast in my own project and it wouldn't be very useful for me (or perhaps anyone) if it had such a limitation.

Is there technical limitation that prevents making receiving calls take non-mut reference &self?

At the moment, no but that's due to the current implementation using interior mutability. The reason I kept the original API (I took over someone's project) was that I thought if we had to the change the implementation for any reason, turning the reference into a mutable one will be a breaking change and a pretty inconvenient one.

However, it's now been years and the implementation has proven to be quite solid by now. Perhaps we can change it now. @notgull any thoughts?

I think at least the documentation should be corrected to describe why async-broadcast differs from async-channel in this aspect.

I don't think this difference is relevant. What users need to know is where they'll use -channel and where they'll use -broadcast and I think that's covered already.

[notgull]

I think it needs to be &mut because the Receiver needs to update its position in the queue:

async-broadcast/src/lib.rs

Line 788 in 74d5889

pos: u64,

...similar to how piper::Reader needs to keep track of its position in the byte slice.

It would be possible to rewrite the implementation to use &self, but it would introduce a lot more interior mutability than there already is. So, I don't think it would be worth it. But it's your call @zeenix

[link2xt]

In my case I don't really need multiple consumers on a single receiver: deltachat/deltachat-core-rust#5478
But because receiver is stored in structures potentially available to multiple threads, I had to wrap Receiver in a Mutex.

I am pretty sure mutex is never locked by multiple threads unless someone uses API in unintended way, e.g. issuing multiple calls to get_next_event() over JSON-RPC API concurrently or something like this, but mutex remains necessary for thread-safety, i.e. to make borrow checker happy.

So it is not a blocker for my use case, but makes async-broadcast not work as a drop-in replacement when someone wants to just replace existing async-channel to add second receiver receiving the same messages (for tests, monitoring etc.). Mutex-wrapped receiver does not work as async-channel receiver, i.e. try_recv now can fail not only when there are no more messages in the queue, but just because other thread has locked the mutex trying to do recv.

[zeenix]

I think it needs to be &mut because the Receiver needs to update its position in the queue:

Ah right. Should have looked more carefully. Thanks. :)

It would be possible to rewrite the implementation to use &self, but it would introduce a lot more interior mutability than there already is. So, I don't think it would be worth it. But it's your call @zeenix

Yeah, me neither. If we can use interior mutability, so can the user and it allows users who don't need interior mutability a a choice and avoid locking costs etc.

So it is not a blocker for my use case, but makes async-broadcast not work as a drop-in replacement when someone wants to just replace existing async-channel

It's not supposed to be a "drop-in" replacement really. I know it would be very convenient if it was but see what I wrote above.