rebus-org / Rebus.AzureServiceBus

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

Avoid potential async deadlock in GetTopicClient #48

Closed jr01 closed 4 years ago

jr01 commented 4 years ago

So we have a (legacy) ASP.net client that uses Rebus. We noticed that sometimes a web api call doesn't return.

I did a memory dump and analysis with windbg and the dumpasync extension revealed that about 20 async http request tasks are stuck for days at GetTopicClient:

000002378c1afea0 <0> Rebus.AzureServiceBus.AzureServiceBusTransport+<GetTopicClient>d__83
.000002378c1b0348 <0> Rebus.AzureServiceBus.AzureServiceBusTransport+<>c__DisplayClass32_0+<<GetOutgoingMessages>b__3>d
..000002378c1b08a0 <0> Rebus.AzureServiceBus.AzureServiceBusTransport+<>c__DisplayClass32_1+<<GetOutgoingMessages>b__1>d
..........

Rebus is awaiting on a Lazy in GetTopicClient .

I'm far from an async guru and did some googling and found the following:

And my conclusion is that an async lazy is a tricky to get right and that a Task.Run should fix it by making sure the code runs on a threadpool thread.


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.

mookid8000 commented 4 years ago

Hi @jr01 , sorry about the slow reply – I've been skiing all week ⛷ putting off work 😅

This is really good information! Good catch, and great that you fixed it. This combination of Lazy and Task is a new thing I learned recently, but I will have to go through all of my usage of this idiom, so I can avoid deadlocks.

Thanks for providing a fix. I'll merge it right away.

mookid8000 commented 4 years ago

Your fix is out in Rebus.AzureServiceBus 7.0.0-a19 on NuGet.org in a few minutes.

Thanks for fixing this!!