Use RunContinuationsAsynchronously in SemaphoreSlim.WaitAsync #22686
Conversation
SemaphoreSlim.Release shouldn't invoke arbitrary continuations as part of its invocation, so when it dequeues a task waiter and goes to complete it, rather than just calling TrySetResult, it queues the task to the thread pool to have TrySetResult invoked there. Now that we have RunContinuationsAsynchronously, though, we can just use that functionality instead. This has two main benefits: 1. We avoid queueing a work item entirely if there are no continuations from the task. This might happen, for example, if the semaphore is released so quickly after waiting on it that the WaitAsync caller hasn't yet hooked up a continuation, in which case the await on the WaitAsync task will just see IsCompleted as true and continue running. 2. We avoid queueing a work item when the task represents a synchronous Wait, which happens if there's already a pending WaitAsync when Wait goes to block. The main benefit here is avoiding potential thread pool starvation, if threads in the pool are blocked in such Waits, and previously another thread pool thread would have been needed to run the queued work item to complete the synchronous Wait.
|
Nice and it will just pick up on ITaskCompletionAction.InvokeMayRunArbitraryCode automatically? |
Exactly. |
|
looks great to me - simple and elegant, thanks |
|
Is this fix also implemented in the .NET Framework ? |
|
This was primarily a perf improvement and it was not backported. |
|
@stephentoub Thanks. It caused the same issues reported by @mgravell at a 3rd party software the company I work in uses , and they've identified it with in their tests. |
|
@pianoman4873 there's a whole bunch of fixes on a wide range of topics: freely and widely available. It is called ".NET Core" (or soon: .NET 5). If you're in .NET Framework, don't expect changes. What works, works. What doesn't: doesn't. |
|
Thanks @mgravell - thanks, but the least I would expect is that after finding such critical bugs and not fixing them is to document them. Without this - the reliability of .NET framework and Microsoft as a company totally diminishes. |
…/coreclr#22686) SemaphoreSlim.Release shouldn't invoke arbitrary continuations as part of its invocation, so when it dequeues a task waiter and goes to complete it, rather than just calling TrySetResult, it queues the task to the thread pool to have TrySetResult invoked there. Now that we have RunContinuationsAsynchronously, though, we can just use that functionality instead. This has two main benefits: 1. We avoid queueing a work item entirely if there are no continuations from the task. This might happen, for example, if the semaphore is released so quickly after waiting on it that the WaitAsync caller hasn't yet hooked up a continuation, in which case the await on the WaitAsync task will just see IsCompleted as true and continue running. 2. We avoid queueing a work item when the task represents a synchronous Wait, which happens if there's already a pending WaitAsync when Wait goes to block. The main benefit here is avoiding potential thread pool starvation, if threads in the pool are blocked in such Waits, and previously another thread pool thread would have been needed to run the queued work item to complete the synchronous Wait. Commit migrated from dotnet/coreclr@e2081d0
SemaphoreSlim.Release shouldn't invoke arbitrary continuations as part of its invocation, so when it dequeues a task waiter and goes to complete it, rather than just calling TrySetResult, it queues the task to the thread pool to have TrySetResult invoked there. Now that we have RunContinuationsAsynchronously, though, we can just use that functionality instead. This has two main benefits:
Fixes https://github.com/dotnet/corefx/issues/35393
cc: @kouvel, @mgravell, @benaadams