CTS.TryReset() concurrency issue #60180

sakno · 2021-10-08T15:10:31Z

sakno
Oct 8, 2021

Probably, there is an issue associated with TryReset even if it is used correctly as described in documentation. It was originally implemented as a part of #50346 and #48492 PRs. I tried to use it to reset CTS in combination with CancelAfter:

Create CTS
Call CancelAfter
Some time later attempt to do TryReset and reuse the instance if method returns true
Nobody else is trying to call Cancel concurrently

This scenario was proposed in original PR as well. But sometimes in very rare situations I see the following exception:

 System.ObjectDisposedException : Cannot access a disposed object.
  Stack Trace:
 at System.Threading.TimerQueueTimer.Change(UInt32 dueTime, UInt32 period)
 at System.Threading.CancellationTokenSource.TryReset()

We see here that the timer has been destroyed but not CTS instance itself. I did investigation and found the following suspicious behavior:

If cancellation requested by the timer then we have potential concurrency between TryReset and NotifyCancellation methods. The last one is called by the timer callback
TryReset successfully passes if (_state == NotCanceledState) check and stops at _timer is not TimerQueueTimer timer code. Now we have the timer saved in the local variable.
In the same time NotifyCancellation is executing from the beginning of the method up to timer.Close();
Now TryReset continues its execution and calls timer.Change(Timeout.UnsignedInfinite, Timeout.UnsignedInfinite)
At this point in time, the timer is closed already and, voila, Change call produces ObjectDisposedException

I can't provide the repro code because the issues is not reproducible constantly (which is normal for concurrency issues). However, the issue is observable for very small cancellation timeouts (1-10 milliseconds).

Proposed fixes: