search

Azure / azure-amqp

AMQP C# library
Other
94 stars 70 forks source link

Use Task.Run instead of thread pool schedule when receiving #191

Closed JoshLove-msft closed 3 years ago

JoshLove-msft commented 3 years ago

Fixes https://github.com/Azure/azure-amqp/issues/190

I only updated this instance as I can consistently reproduce the issue I was running into in Service Bus by making this one change. I can update other instances of ActionItem.Schedule if you would like.

JoshLove-msft commented 3 years ago

I found this article that seems to describe the situation we are running into - https://medium.com/criteo-engineering/net-threadpool-starvation-and-how-queuing-makes-it-worse-512c8d570527

In particular, using QueueUserWorkItem means that the work is queued to the global queue rather than the thread's local queue. If you read through the scenario, it explains why this might cause starvation when async code is being waited on synchronously (sync over async). The ideal fix would be to root out any place where we are doing sync over async.

danielmarbach commented 3 years ago

See also https://github.com/Azure/azure-amqp/pull/188 which would address this and safe a few allocs. Apparently though we need to preserve the crash scenario of the thread pool :(

xinchen10 commented 3 years ago

Some code, most likely from the user of the library, is blocking the threads. Please debug and fix such code. For thread pool scheduling, local vs global, we will use #188 for further evaluation.