Skip to content
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.

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

CTS.TryReset() concurrency issue #60182

Closed
sakno opened this issue Oct 8, 2021 · 8 comments · Fixed by #60298 or #60323
Closed

CTS.TryReset() concurrency issue #60182

sakno opened this issue Oct 8, 2021 · 8 comments · Fixed by #60298 or #60323
Assignees
Milestone

Comments

@sakno
Copy link
Contributor

sakno commented Oct 8, 2021

Description

A full description of the issue is here: #60180

Reproduction Steps

  1. Create CTS
  2. Call CancelAfter with very small timeout (1-10 milliseconds)
  3. Call TryReset concurrently with CancelAfter

Expected behavior

TryReset should normally handle the concurrency with timer cancellation and return true/false.

Actual behavior

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

Regression?

No, TryReset is introduced in .NET 6.

Known Workarounds

Do not use TryReset at all and re-create CTS every time when needed.

or

catch ObjectDisposedException by the caller:

static bool TryResetSafe(CancellationTokenSource source)
{
  try
  {
    return source.TryReset();
  }
  catch (ObjectDisposedException)
  {
    return false;
  }
}

Configuration

The environment is not relevant. .NET SDK version is 6.0.100-rc.1.21458.32

Other information

No response

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Oct 8, 2021
@stephentoub stephentoub self-assigned this Oct 8, 2021
@ghost
Copy link

ghost commented Oct 8, 2021

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

A full description of the issue is here: #60180

Reproduction Steps

  1. Create CTS
  2. Call CancelAfter with very small timeout (1-10 milliseconds)
  3. Call TryReset concurrently with CancelAfter

Expected behavior

TryReset should normally handle the concurrency with timer cancellation and return true/false.

Actual behavior

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

Regression?

No, TryReset is introduced in .NET 6.

Known Workarounds

Do not use TryReset at all and re-create CTS every time when needed.

Configuration

The environment is not relevant. .NET SDK version is 6.0.100-rc.1.21458.32

Other information

No response

Author: sakno
Assignees: stephentoub
Labels:

area-System.Threading, untriaged

Milestone: -

@stephentoub stephentoub added bug and removed untriaged New issue has not been triaged by the area owner labels Oct 8, 2021
@stephentoub stephentoub added this to the 6.0.0 milestone Oct 8, 2021
@sakno
Copy link
Contributor Author

sakno commented Oct 9, 2021

@stephentoub , I pushed draft PR but couldn't test it because of changes in CoreCLR codebase.

/projects/dotnet/src/coreclr/jit/morph.cpp:67:35: error: assigning to 'var_types' from incompatible type 'unsigned char'
      call->gtReturnType          = static_cast<unsigned char>(tree->TypeGet());

@sakno
Copy link
Contributor Author

sakno commented Oct 9, 2021

My bad, sorry. I changed target branch from main to release/6.0-rc2.

sakno added a commit to sakno/runtime that referenced this issue Oct 12, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Oct 12, 2021
@sakno
Copy link
Contributor Author

sakno commented Oct 12, 2021

@stephentoub , two PRs are here as discussed previously:

@stephentoub
Copy link
Member

thanks!

sakno added a commit to dotnet/dotNext that referenced this issue Oct 12, 2021
stephentoub pushed a commit that referenced this issue Oct 13, 2021
* Quick fix of #60182

* Fixed reset condition when there is no timer

* Simplify branching
github-actions bot pushed a commit that referenced this issue Oct 13, 2021
stephentoub pushed a commit that referenced this issue Oct 13, 2021
* Quick fix of #60182

* Fixed reset condition when there is no timer

* Simplify branching

Co-authored-by: sakno <roman.sakno@gmail.com>
@danmoseley
Copy link
Member

@stephentoub is there remaining 6.0 work here? or can we close now?

@stephentoub
Copy link
Member

stephentoub commented Oct 21, 2021

is there remaining 6.0 work here?

No, #60323 should have closed it. Not sure why it didn't; maybe that bot doesn't have perms to do so.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants