Fix hang in HTTP/2 handling of canceling credit waits (#40243)
authorStephen Toub <stoub@microsoft.com>
Tue, 4 Aug 2020 14:31:40 +0000 (10:31 -0400)
committerGitHub <noreply@github.com>
Tue, 4 Aug 2020 14:31:40 +0000 (10:31 -0400)
commitc469ac9f3a9687bc118d899047a166e327e2a33e
treeddf1fb973dbdad7b694310e58ef820640831ea1f
parentc0707135c4969be520efae02c9593d481c57cc8f
Fix hang in HTTP/2 handling of canceling credit waits (#40243)

* Re-combine CreditWaiter and CancelableCreditWaiter

The split was to optimize the case when there's no cancellation token, but based on the evolution of the codebase, that doesn't really happen, and it makes the common cancellation case a bit more expensive (and convoluted).

* Fix hang in HTTP/2 handling of canceling credit

The CreditWaiter took a lock to protect its ManualResetValueTaskSourceCore state.  The lock was also shared with its parent.  This could result in a situation where the parent was holding the lock and called into the CreditWaiter to dispose of it and in turn dispose of the registration with the CancellationToken.  That registration could have represented an in-flight cancellation callback, which would also try to take the lock in order to complete the MRVTSC.  And since CancellationTokenRegistration.Dispose blocks waiting for an in-flight cancellation callback to complete, this could deadlock, with a thread holding the lock while waiting for another thread that tries to take the lock to make forward progress.

This fixes it simply by avoiding the lock entirely and instead using a simple compare-exchange to protect the right to complete the MRVTSC.

* Address PR feedback
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/CreditManager.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/CreditWaiter.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs
src/libraries/System.Net.Http/tests/StressTests/HttpStress/StressServer.cs