AsyncSemaphore deadlock: EnterAsync holds lock while blocking on cancellation handler which needs lock

[jasonmalinowski]

(This is being reported from the analysis from devdiv-1496681

Bug description

AsyncSemaphore.EnterAsync has this specific line:

vs-threading/src/Microsoft.VisualStudio.Threading/AsyncSemaphore.cs

Line 144 in 683b45c

info.Cleanup();

Which calls:

vs-threading/src/Microsoft.VisualStudio.Threading/AsyncSemaphore.cs

Line 356 in 683b45c

cancellationTokenRegistration.Dispose();

If you get particularly unlucky with your timing, that call to cancellationTokenRegistration.Dispose() will block on the cancellation handler completing; if that's running on another thread and potentially blocked in it's own right, then the monitor is held and all other calls to AsyncSemaphore will block, synchronously.

I admit it's not immediately obvious to me if this will deadlock permanently:

vs-threading/src/Microsoft.VisualStudio.Threading/AsyncSemaphore.cs

Line 222 in 683b45c

lock (waiterInfo.Owner.syncObject)

might be trying to acquire the monitor as well in the cancellation handler -- we might have a case where the cancellation handler is acquiring the monitor and the monitor is blocking on the cancellation handler. But my analysis might be incorrect there.

I suspect one fix is to move info.Cleanup() outside the monitor lock, but not sure if other fixes are available too.

Repro steps

Have code using AsyncSemaphore with cancellation, and get unlucky.

[AArnott]

More notes from @jasonmalinowski's investigation:

404 threads are blocked trying to acquire the monitor in AsyncSemaphore.EnterAsync.
The one thread that does have the monitor is in here:

>	mscorlib.dll!System.Threading.SpinWait.SpinOnce() Line 146	C#
 	mscorlib.dll!System.Threading.CancellationTokenSource.WaitForCallbackToComplete(System.Threading.CancellationCallbackInfo callbackInfo) Line 946	C#
 	mscorlib.dll!System.Threading.CancellationTokenRegistration.Dispose() Line 90	C#
 	Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.AsyncSemaphore.WaiterInfo.Cleanup() Line 357	C#
 	Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.AsyncSemaphore.EnterAsync(System.TimeSpan timeout, System.Threading.CancellationToken cancellationToken) Line 147	C#

22 threads are here in StreamJsonRpc:

 	mscorlib.dll!System.Threading.SpinWait.SpinOnce() Line 146	C#
 	mscorlib.dll!System.Threading.CancellationTokenSource.WaitForCallbackToComplete(System.Threading.CancellationCallbackInfo callbackInfo) Line 946	C#
 	mscorlib.dll!System.Threading.CancellationTokenRegistration.Dispose() Line 90	C#
 	mscorlib.dll!System.Threading.CancellationTokenSource.Dispose(bool disposing) Line 582	C#
>	StreamJsonRpc.dll!StreamJsonRpc.JsonRpc.InvokeCoreAsync(StreamJsonRpc.Protocol.JsonRpcRequest request, System.Type expectedResultType, System.Threading.CancellationToken cancellationToken) Line 1906	C#

22 other threads (suspicious the same number?) are here:

>	Microsoft.VisualStudio.Threading.dll!Microsoft.VisualStudio.Threading.AsyncSemaphore.CancellationHandler(object ste) Line 234	C#