* Fix race condition in ManualResetValueTaskSourceCore
ManualResetValueTaskSourceCore.GetStatus is used from ValueTaskAwaiter.IsCompleted. If GetStatus returns Success too early, then IsCompleted will also return true too early, which will result in GetResult being called too early. This doesn't happen today when an MRVTSC is used sequentially. But if an instance is pooled by the object its stored on getting put back into a pool as part of a call to the wrapper's GetResult, then we can end up in the following situation:
- Thread 1 starts an await on an instance containing an MRVTSC. It calls IsCompleted.
- Thread 2 starts to complete that instance, getting as far as calling SignalCompletion and setting _completed to true but not yet setting _continuation.
- Thread 1 sees _completed == true and returns true from IsCompleted. It calls GetResult, and the wrapper extracts the result from the instance, resets it, and puts it back into the pool.
- Thread 3 takes the object out of the pool and starts using it.
- Thread 2 continues SignalCompletion on that instance, and sets _continuation to the sentinel.
- Now Thread 3's instance's _continuation is the sentinel when it should be null. If it calls SignalCompletion, it'll erroneously find that _continuation is not null and will queue _continuation/_continuationState for execution, resulting in the sentinel getting executed. If it calls OnCompleted, it'll find that the _continuation is not null, and will queue the supplied continuation/state to execute immediately even though the operation may not yet actually be done.
The fix is simply to check not just _completed but also _continuation. The operation is considered pending if either _completed is false, meaning SignalCompletion has not yet been called, or if _continuation is still null, meaning it's neither been set to the supplied delegate nor to the sentinel. We can't just rely on _completed for the above outlined reasons, and we can't just rely on _continuation because it can be non-null if OnCompleted was called to hook up a callback (if we only cared about the await pattern, then we could just check _continuation and wouldn't need _completed, but we also need to support non-await access).
* Address PR feedback
{
ValidateToken(token);
return
- !_completed ? ValueTaskSourceStatus.Pending :
+ _continuation == null || !_completed ? ValueTaskSourceStatus.Pending :
_error == null ? ValueTaskSourceStatus.Succeeded :
_error.SourceException is OperationCanceledException ? ValueTaskSourceStatus.Canceled :
ValueTaskSourceStatus.Faulted;