Using of CancellationToken in RemoteJSDataStream

[leeriorio]

I analyzed the source code of ASP.NET Core v6.0.18 using a Svace static analyzer. He found an error of category HANDLE_LEAK with the following message

CancellationTokenSource.CreateLinkedTokenSource(a, b) is not disposed at the end of the function

in method GetLinkedCancellationToken(). Here's a source

https://github.com/dotnet/aspnetcore/blob/28b2bfd3ac67f07a5985550f1bec2e659af02aea/src/Components/Server/src/Circuits/RemoteJSDataStream.cs#L190-L202

First of all, its useful to note, that method CancellationTokenSource.CreateLinkedTokenSource() (link below) already has its own all necessary checks that are implemented in the method above

https://github.com/dotnet/corert/blob/master/src/System.Private.CoreLib/shared/System/Threading/CancellationTokenSource.cs#L748

Moreover, calling CreateLinkedTokenSource() actually creates instance of TokenSource, which can be disposed

Proposed update

This method is applied only twice in this class: in methods

https://github.com/dotnet/aspnetcore/blob/28b2bfd3ac67f07a5985550f1bec2e659af02aea/src/Components/Server/src/Circuits/RemoteJSDataStream.cs#L178-L182

and

https://github.com/dotnet/aspnetcore/blob/28b2bfd3ac67f07a5985550f1bec2e659af02aea/src/Components/Server/src/Circuits/RemoteJSDataStream.cs#L184-L188

and I think, these methods can be improved with using using construction and removing GetLinkedCancellationToken() like this:

public override async Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
{
    using var linkedCancellationToken = CancellationTokenSource.CreateLinkedTokenSource(_streamCancellationToken, cancellationToken).Token;
    return await _pipeReaderStream.ReadAsync(buffer.AsMemory(offset, count), linkedCancellationToken);
}   

public override async ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default)
{
    using var linkedCancellationToken = CancellationTokenSource.CreateLinkedTokenSource(_streamCancellationToken, cancellationToken).Token;
    return await _pipeReaderStream.ReadAsync(buffer, linkedCancellationToken);
}

Found by Linux Verification Center (linuxtesting.org) with SVACE. Reporter: Aleksey Kolosov (kolosov.ai@npc-ksb.ru). Organization: info@npc-ksb.ru

[ghost]

To learn more about what this message means, what to expect next, and how this issue will be handled you can read our Triage Process document. We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. Because it's not immediately obvious what is causing this behavior, we would like to keep this around to collect more feedback, which can later help us determine how to handle this. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact work.

[ghost]

To learn more about what this message means, what to expect next, and how this issue will be handled you can read our Triage Process document. We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. Because it's not immediately obvious what is causing this behavior, we would like to keep this around to collect more feedback, which can later help us determine how to handle this. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact work.

[mkArtakMSFT]

@MackinnonBuck parking this in .NET 9 Planning and assigning to you to spend a few hours to evaluate the ask and see if it's actually an issue and is beneficial.

[MackinnonBuck]

@mkArtakMSFT This may be a real leak, and the proposed change seems close to what we'd want to do to address it. If we were to receive a PR with the fix, I believe we'd be inclined to take it.