Multi-threading behavior with v7 client

[qrli]

In previous versions, the doc states that a model/channel must be only accessed from one thread at a time.

In the v7 implementation, I see that the .NET Channel classes are used internally and there is an internal thread to actually process and send commands to server. My gut-feeling is that now it is safe for my code to call methods on a RabbitMQ model/channel object from different threads without locks in my code. On the other hand, the async API makes it impractical to do locks, where the lock will span over awaits which is a recipe for deadlock.

Just want to confirm whether my understanding is correct. Thanks in advance.

[lukebakken]

On the other hand, the async API makes it impractical to do locks, where the lock will span over awaits which is a recipe for deadlock.

Use SemaphoreSlim instead of lock. This code base uses SemaphoreSlim in several places.

My gut-feeling is that now it is safe for my code to call methods on a RabbitMQ model/channel object from different threads without locks in my code.

What would be extremely useful to this project is for you to use the latest RC (https://www.nuget.org/packages/RabbitMQ.Client/7.0.0-rc.8), remove the locks from your code, and report back.

After a quick review of the SocketFrameHandler class, I think you're correct. I will add some tests to confirm.

[qrli]

Use SemaphoreSlim instead of lock. This code base uses SemaphoreSlim in several places.

Ahh, that's great to know. I'll have a try.

[michaelklishin]

For multi-frame operations (namely basic.publish) and synchronous (as in, having a protocol response) protocol methods (e.g. queue.declare or queue.bind) it is a extremely likely that even with socket-level synchronization in the library you will run into obscure issues, async API or not.

Bunny (the most popular Ruby client) does exactly this: it synchronizes socket writes because not doing so is not practical in the context of certain Web servers, but sharing channels between threads is not supported. Two levels of locking on the hot code path and the complexity of a concurrent continuation implementation (in Ruby, it can be much easier to reason about in C#) were not worth the effort. My point is that socket-level synchronization is not enough in practice with key operations mentioned above.

[danielmarbach]

Besides having a dedicated connection for publishers and consumers, have we ever settled on the guidance around sharing the publisher channel across multiple concurrent operations? According to my knowledge and also mentioned again by @lukebakken this should be fine since the publish operations are being added to the socket frame handler using a System.Threading.Channel and then dispatched.

That would indicate as long as you have consuming, admin and publishing separated there should be no interference happening and there is no reason to have multiple pooled channels anymore for the "concurrent" publishes. Should the guidance be refined?

[lukebakken]

When someone ask I tell them that the IChannel instances are thread-safe. I'm assuming that you refer to this web page when it comes to "official guidance"?

[SaifAqqad]

That would indicate as long as you have consuming, admin and publishing separated there should be no interference happening and there is no reason to have multiple pooled channels anymore for the "concurrent" publishes. Should the guidance be refined?

This should be mentioned clearly in the dotnet client docs, as the current state of the client docs + rabbitmq server/generic docs have very conflicting guidance

I would also suggest adding more examples that implement this usage in a more realistic scenario (for example in an ASP.NET Core web API)

[lukebakken]

Please take the time to open pull requests to do the work you are suggesting. If you don't have time to do the work yourself, at least point out exactly where the issues are, rather than make statements such as this:

as the current state of the client docs + rabbitmq server/generic docs have very conflicting guidance

I would also suggest adding more examples that implement this usage in a more realistic scenario (for example in an ASP.NET Core web API)

Please feel free to do this yourself. The tutorial code can be found here:

https://github.com/rabbitmq/rabbitmq-tutorials/tree/main/dotnet

...and the associated website tutorials can be found here:

https://github.com/rabbitmq/rabbitmq-website/tree/main/client-libraries

https://github.com/rabbitmq/rabbitmq-website/tree/main/tutorials

My time to focus on the .NET client is extremely limited these days, so anything that the community can do on its own is appreciated.

[SaifAqqad]

If you don't have time to do the work yourself, at least point out exactly where the issues are

This is from the dotnet client docs:

This is a hard requirement for publishers: sharing a channel (an IChannel instance) for concurrent publishing will lead to incorrect frame interleaving at the protocol level. Channel instances must not be shared by threads that publish on them.

However, based on the comments made in this discussion, it seems v7 does not require separate (or pooled) channels for concurrent publishes, so the docs should probably be updated.

---

rather than make statements such as this

I'm sorry, my comment was not directed at the maintainers or anyone specific, it was just a general suggestion.