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

CancellationTokenTaskSource's dispose #272

Open
yfital opened this issue Mar 12, 2023 · 0 comments
Open

CancellationTokenTaskSource's dispose #272

yfital opened this issue Mar 12, 2023 · 0 comments

Comments

@yfital
Copy link

yfital commented Mar 12, 2023

Hey,

I'm wondering whether the current behavior is correct:

        public void Dispose()
        {
            _registration?.Dispose();
        }

When we use it (maybe our usage is different) we usually use this to hook a CT towards a WhenAll.
So for instance, we'll have some sort of:

async Task MyWhenAll(tasks[], ct)
{
using var ctts = new CTTS(ct);
try 
{ 
    await await WhenAny(WhenAll(tasks), ctts.task);
}
catch
{ // handle specifics if needed
}

if ( ctts.task.iscompleted )
    return; // handle timeout

//all valid
    return;
} 

The disposal currently disposes of the registration, meaning it detaches the TCS from the CT.
But the TCS is now a long-living task that is just kept in a non-finite state.

In case everything worked as expected (no exceptions), the dispose should have marked the TCS as completed, no?

That will also supported the case where someone used CTTS in a scope and disposed it, which I expected to have the TCS be marked as completed and free the WhenAll.
(Not because I think someone should do it, but for consistency with the language).

To clarify, I do not think there is a leak here, it's merely an optimization/consistency look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant