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

is not disposing of CancellationTokenSource in ToCancellationToken a problem? #4611

Open
SimonCropp opened this issue Mar 23, 2025 · 2 comments · May be fixed by #4625
Open

is not disposing of CancellationTokenSource in ToCancellationToken a problem? #4611

SimonCropp opened this issue Mar 23, 2025 · 2 comments · May be fixed by #4625

Comments

@SimonCropp
Copy link
Contributor

given CancellationTokenSource is disposable. is it a problem to not have it being disposed?

public static class TimeSpanExtensions
{
  public static CancellationToken ToCancellationToken(this TimeSpan timeout)
  {
    var cts = new CancellationTokenSource(timeout);
    return cts.Token;
  }
}

https://learn.microsoft.com/en-us/dotnet/standard/threading/cancellation-in-managed-threads

You should be sure to call the CancellationTokenSource.Dispose method when you have finished using the cancellation token source to free any unmanaged resources it holds.

@rockfordlhotka
Copy link
Member

It does sound like it should be disposed. This extension method can't do it, so it is the responsibility of any code using the token.

It will be interesting to see if all the places it is used are in code where dispose is an option. If not, we'll have to consider removing the option. Hopefully that's not an issue, as it should be fine in things like data portal proxies and other DI-provided services. My one concern is if this has crept into the async rules engine, as none of those things are currently disposable (afaik).

@StefanOssendorf
Copy link
Contributor

The services don't have to be disposable to use the cts. But we should make sure to dispose it in our own code.
I think I'll take a look this weekend. So I'm assigning myself to it.

@StefanOssendorf StefanOssendorf self-assigned this Mar 25, 2025
StefanOssendorf added a commit that referenced this issue Mar 29, 2025
…ernalize them in v10)

Add new method ToCancellationTokenSource and dispose the source correctly.

Fixes #4611
rockfordlhotka pushed a commit that referenced this issue Mar 31, 2025
* Make ToCancellationToken and containing class obsolete (so we can internalize them in v10)
Add new method ToCancellationTokenSource and dispose the source correctly.

Fixes #4611

* Revert unnecessary await
StefanOssendorf added a commit that referenced this issue Apr 4, 2025
Make Csla internal visible to Csla.Blazor for reuse

Fixes #4611
@StefanOssendorf StefanOssendorf linked a pull request Apr 4, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

3 participants