Skip to content

Fix: CTS.TryReset() concurrency issue #60182 #60224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stephentoub merged 3 commits into from
Oct 13, 2021
Merged

Fix: CTS.TryReset() concurrency issue #60182 #60224

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7913094
Quick fix of #60182
sakno Oct 12, 2021
c398288
Fixed reset condition when there is no timer
sakno Oct 12, 2021
096837e
Simplify branching
sakno Oct 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions src/libraries/System.Private.CoreLib/src/System/Threading/CancellationTokenSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,13 +392,24 @@ public bool TryReset()
// to transition from canceled to non-canceled.
if (_state == NotCanceledState)
{
// If there is no timer, then we're free to reset. If there is a timer, then we need to first try
// to reset it to be infinite so that it won't fire, and then recognize that it could have already
// fired by the time we successfully changed it, and so check to see whether that's possibly the case.
// If we successfully reset it and it never fired, then we can be sure it won't trigger cancellation.
bool reset =
_timer is not TimerQueueTimer timer ||
(timer.Change(Timeout.UnsignedInfinite, Timeout.UnsignedInfinite) && !timer._everQueued);
bool reset = false;

try
{
// If there is no timer, then we're free to reset. If there is a timer, then we need to first try
// to reset it to be infinite so that it won't fire, and then recognize that it could have already
// fired by the time we successfully changed it, and so check to see whether that's possibly the case.
// If we successfully reset it and it never fired, then we can be sure it won't trigger cancellation.
reset = _timer is not TimerQueueTimer timer ||
(timer.Change(Timeout.UnsignedInfinite, Timeout.UnsignedInfinite) && !timer._everQueued);
}
catch (ObjectDisposedException)
{
// Just eat the exception. There is no other way to tell that
// the timer has been disposed, and even if there were, there
// would not be a good way to deal with the observe/dispose
// race condition.
}

if (reset)
{
Expand Down