Fix CountdownEvent.Wait to respect underlying event's status (dotnet/corefx#35024)

A common pattern with CountdownEvent is:
```C#
ce.Reset(count);
.. // launch count operations that'll all call ce.Signal()
ce.Wait();
```
and to use that repeatedly.  Reset is explicitly not a thread-safe operation, but in a pattern like this, it should be acceptable as long as the only operations signaling the event are those launched here, since Wait should only return when they've all completed.  However, CountdownEvent.Wait is currently checking CountdownEvent.IsSet, which returns true if the count has reached 0, and the CountdownEvent.Signal method first decrements the count and the sets the underlying event if it reaches 0.  That means there's a window where the count is 0 but the event hasn't been set, which means if Wait is called in that window, it could return successfully even though an operation is still in flight invoking Signal, which then in turn is problematic as that could race with a subsequent call to Reset.

This fixes that by checking this._event.IsSet instead of this.IsSet.

Commit migrated from https://github.com/dotnet/corefx/commit/66403fb0e4c9e3c05e23db23c04742b8a50dbe0a

diff --git a/src/libraries/System.Threading/src/System/Threading/CountdownEvent.cs b/src/libraries/System.Threading/src/System/Threading/CountdownEvent.cs
index 75eac66..8c054b6 100644 (file)
--- a/src/libraries/System.Threading/src/System/Threading/CountdownEvent.cs
+++ b/src/libraries/System.Threading/src/System/Threading/CountdownEvent.cs
@@ -100,7 +100,7 @@ namespace System.Threading
             get
             {
                 // The latch is "completed" if its current count has reached 0. Note that this is NOT
-                // the same thing is checking the event's IsCompleted property. There is a tiny window
+                // the same thing is checking the event's IsSet property. There is a tiny window
                 // of time, after the final decrement of the current count to 0 and before setting the
                 // event, where the two values are out of sync.
                 return (_currentCount <= 0);
@@ -542,7 +542,11 @@ namespace System.Threading
             ThrowIfDisposed();
             cancellationToken.ThrowIfCancellationRequested();
 
-            bool returnValue = IsSet;
+            // Check whether the event is already set.  This is checked instead of this.IsSet, as this.Signal
+            // will first decrement the count and then if it's 0 will set the event, thus it's possible
+            // we could observe this.IsSet as true while _event.IsSet is false; that could in turn lead
+            // a caller to think it's safe to use Reset, while an operation is still in flight calling _event.Set.
+            bool returnValue = _event.IsSet;
 
             // If not completed yet, wait on the event.
             if (!returnValue)