rebus-org / Rebus.AzureServiceBus

:bus: Azure Service Bus transport for Rebus
https://mookid.dk/category/rebus
Other
33 stars 20 forks source link

Add ConfigureAwait to fix deadlock issues. #8

Closed lezzi closed 6 years ago

lezzi commented 6 years ago

We got multiple deadlock issues using Rebus 4.2.1. After an hour of debugging I realized that Rebus.AzureServiceBus does not use ConfigureAwait at all.

Publish method of rebus hangs during GetOutgoingMessages call. Of course it happens only in specific types of applications that have synchronization context and when we use task.Wait() or task.Result.


Rebus is MIT-licensed. The code submitted in this pull request needs to carry the MIT license too. By leaving this text in, I hereby acknowledge that the code submitted in the pull request has the MIT license and can be merged with the Rebus codebase.

CLAassistant commented 6 years ago

CLA assistant check
All committers have signed the CLA.

mookid8000 commented 6 years ago

Hi there @lezzi , it's great you went through Rebus.AzureServiceBus and added .ConfigureAwait(false) to all awaited calls.... but I don't understand how that fixes your deadlock if you

bus.Publish(something).Wait();

The calling thread will still be blocked, right?


Btw Rebus has a sync API, which is specifically meant to handle your scenario – if you are forced into being synchronous, you can get a sync API here:

var syncBus = bus.Advanced.SyncBus;

where all methods of IBus exist in a synchronous implementation.

mookid8000 commented 6 years ago

Excellent work btw. – it's merged now, and it will be out in Rebus.AzureServiceBus 4.0.1 when the buildserver has done its thing 👍

lezzi commented 6 years ago

Hey @mookid8000. Let me explain a little bit.

It happens only in project types that have synchronization context by default. Like Asp MVC, WPF and so on. When we call bus.Publish(something).Wait() thread is blocked, correct. But it means that inside Publish implementation, if you call any async method using await without ConfigureAwait(false), after completion it will try to return to original thread, which is blocked by Wait().

In our solution we had a situation where we were forced to call Wait for some outdated code. Refactoring all that code is complicated and will trigger a significant amount of changes.

Honestly, didn't know about sync Rebus implementation. In theory we could try to inject sync implementations for old code and async for new code. But in our case wait is not called for publishing directly. It is somewhere in call stack, which makes almost impossible to determine if we need to call async or sync bus.

To wrap this up, I would say it is better to always use ConfigureAwait in libraries like this. You never know how your async methods will be called and ideally it should work everywhere if it is possible.

lezzi commented 6 years ago

Btw, @mookid8000, when is new version published to Nuget usually?

mookid8000 commented 6 years ago

Damn! For some reason it wasn't pushed.... just a moment

mookid8000 commented 6 years ago

ah, sorry – the Rebus.AzureServiceBus repository apparently contained an older version of NuGet.exe

image

Updating it solved it – it has been pushed now 😄

Thanks for reminding me 👍