Avoid a couple theoretical reorderings in ManualResetValueTaskSourceCore (#72657)
authorStephen Toub <stoub@microsoft.com>
Wed, 27 Jul 2022 00:52:40 +0000 (20:52 -0400)
committerGitHub <noreply@github.com>
Wed, 27 Jul 2022 00:52:40 +0000 (20:52 -0400)
In a typical usage sequence, GetStatus is called and then GetResult is called.  If the read of _result in GetResult could move to before the read of _continuation in GetStatus, then there could be a race condition where _result is read prior to it being written, then _continuation is set as part of the operation completing, and the implementation ends up returning an erroneous result.  Similarly, in SignalCompletion, if the read of _continuationState could moved to before the first read of _continuation, it could be stale.

src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Sources/ManualResetValueTaskSourceCore.cs

index 2ce2087..a7341ce 100644 (file)
@@ -79,7 +79,7 @@ namespace System.Threading.Tasks.Sources
         {
             ValidateToken(token);
             return
-                _continuation == null || !_completed ? ValueTaskSourceStatus.Pending :
+                Volatile.Read(ref _continuation) == null || !_completed ? ValueTaskSourceStatus.Pending :
                 _error == null ? ValueTaskSourceStatus.Succeeded :
                 _error.SourceException is OperationCanceledException ? ValueTaskSourceStatus.Canceled :
                 ValueTaskSourceStatus.Faulted;
@@ -213,11 +213,13 @@ namespace System.Threading.Tasks.Sources
             }
             _completed = true;
 
-            if (_continuation is null && Interlocked.CompareExchange(ref _continuation, ManualResetValueTaskSourceCoreShared.s_sentinel, null) is null)
+            if (Volatile.Read(ref _continuation) is null && Interlocked.CompareExchange(ref _continuation, ManualResetValueTaskSourceCoreShared.s_sentinel, null) is null)
             {
                 return;
             }
 
+            Debug.Assert(_continuation is not null);
+
             if (_executionContext is null)
             {
                 if (_capturedContext is null)