Should try_recv on a Reciver take &mut self or &self

help
#1

Hi,
I’ve implemented a queue which internally uses a vector as a buffer, where writers and readers have their own indexes, which they increment upon each send/recv operation. I’ve modeled the API after the std::sync::mpsc, and in it the try_recv method takes a immutable reference to self. To make this work in my queue I had to use AtomicUsize.

But now that I think about it there is no reason that I know of for the try_recv function to be immutable.
So the question is: Is there a reason for try_recv to be immutable, and if so what would you suggest I do since the receiver increments it’s index on each try_recv call

#2

It’s better to think of &/&mut not as immutable vs mutable, but whether it allows shared or requires exclusive access.

Do you require that there has to be only one call to try_recv at a time? If not, it’s fine to allow shared access.

In general, if you can, it’s fine to make things take a shared reference, since these are much more flexible and easier to work with by users of your library.

2 Likes
#3

in my implementation neither Receiver nor the Sender should be concurrently accessed by multiple threads. They themselves provide an interface to a “channel”.
But that also applies to std::sync::mpsp and crossbeam::channel, but in both cases the send and recv methods accept immutable references to self.

If it’s just for convenience’s sake, then I am fine with my implementation having a stricter API.

#4

If you believe taking &self is safe, then do it. No reason to arbitrarily require &mut self if it isn’t actually required.

I think mpsc::Receiver is protected from multiple calls to try_recv by Receiver not implementing Sync. (impl<T> !Sync for Receiver<T> in https://doc.rust-lang.org/std/sync/mpsc/struct.Receiver.html).

The only reasons you would want to take &mut self instead of &self are

  1. if your code is unsound if it were implemented using &self. In particular, interior mutability on anything outside an UnsafeCell or atomic is unsound, so if taking &self requires you to do that then that’s a good reason to take &mut self instead.
  2. if &self allows users to use it in a way you believe will be incorrect the majority of the time.

I don’t see either of these being true, so I would just keep &self. But I haven’t looked at the code in depth so my assessment could be incorrect.

Edit: It looks like BareSubscriber is sync (impl<T> Sync for BareSubscriber<T> is in the documentation), so your current method taking &self will allow two threads to call try_recv at the same time. If you think this will cause problems, even if it doesn’t cause unsoundness, I would recommend taking &mut self. If it will do the right thing no matter how many threads are accessing it concurrently, then no matter. Since Subscriber is Clone, I assume that the latter is true.