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

[Bug] Background proactive token refresh is not properly canceled #4473

Closed
pmaytak opened this issue Dec 15, 2023 · 1 comment · Fixed by #4471
Closed

[Bug] Background proactive token refresh is not properly canceled #4473

pmaytak opened this issue Dec 15, 2023 · 1 comment · Fixed by #4471

Comments

@pmaytak
Copy link
Contributor

pmaytak commented Dec 15, 2023

Library version used

4.58.1

.NET version

Any

Scenario

ConfidentialClient - web api (AcquireTokenOnBehalfOf), ConfidentialClient - service to service (AcquireTokenForClient), ManagedIdentityClient - managed identity

Is this a new or an existing app?

None

Issue description and reproduction steps

Proactive token refresh is done on a separate thread. A cancellation token provided by the user is passed into it. If the parent token source is disposed after the original token request completes, then any usage of that cancellation token in the proactive refresh task will cause a "object is disposed" exception.

Relevant code snippets

No response

Expected behavior

No response

Identity provider

Microsoft Entra ID (Work and School accounts and Personal Microsoft accounts)

Regression

No response

Solution and workarounds

Create a linked cancellation token source using the parent user-provided cancellation token to be used in the background refresh task.

@pmaytak pmaytak added this to the 4.59.0 milestone Dec 15, 2023
@pmaytak pmaytak self-assigned this Dec 15, 2023
@pmaytak pmaytak changed the title [Bug] Background proactive token refresh is not properly cancelled [Bug] Background proactive token refresh is not properly canceled Dec 21, 2023
@pmaytak
Copy link
Contributor Author

pmaytak commented Dec 21, 2023

After some investigation, Seems like this issue may happen only under rare circumstances. When CancellationTokenSource is disposed, then calling its properties, including Token, will result in an ObjectDisposedException. However, since the token is passed into the inner methods, the token and it's value properties like CanBeCanceled and IsCancellationRequested can still be called. The only exception to this is the WaitHandle property is disposed. However, it doesn't seem to be used anywhere in the MSAL code.

Small repro:

public static async Task Main(string[] args)
{
    var cts = new CancellationTokenSource();
    Method1(cts, cts.Token);
}
public static void Method1(CancellationTokenSource source, CancellationToken cancellationToken)
{
    source.Dispose();
    var canceled = cancellationToken.IsCancellationRequested;
    cancellationToken.ThrowIfCancellationRequested();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
1 participant