Skip to content

Adds Disposal Logic for IJSStreamReference in PullFromJSDataStream #64426

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+72 −7
Open

Adds Disposal Logic for IJSStreamReference in PullFromJSDataStream #64426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1817f2
Refactor PullFromJSDataStream disposal logic and add unit test for Di…
Nov 18, 2025
64a0116
Nov 18, 2025
750c9b3
Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 50 additions & 6 deletions src/Components/Shared/src/PullFromJSDataStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ internal sealed class PullFromJSDataStream : Stream
private readonly IJSRuntime _runtime;
private readonly IJSStreamReference _jsStreamReference;
private readonly long _totalLength;
private readonly CancellationToken _streamCancellationToken;
private readonly CancellationTokenSource _streamCts;
private long _offset;
private bool _isDisposed;

public static PullFromJSDataStream CreateJSDataStream(
IJSRuntime runtime,
Expand All @@ -36,8 +37,9 @@ private PullFromJSDataStream(
_runtime = runtime;
_jsStreamReference = jsStreamReference;
_totalLength = totalLength;
_streamCancellationToken = cancellationToken;
_streamCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
_offset = 0;

}

public override bool CanRead => true;
Expand Down Expand Up @@ -88,7 +90,7 @@ public override async ValueTask<int> ReadAsync(Memory<byte> buffer, Cancellation
private void ThrowIfCancellationRequested(CancellationToken cancellationToken)
{
if (cancellationToken.IsCancellationRequested ||
_streamCancellationToken.IsCancellationRequested)
_streamCts.IsCancellationRequested)
{
throw new TaskCanceledException();
}
Expand All @@ -104,10 +106,52 @@ private async ValueTask<byte[]> RequestDataFromJSAsync(int numBytesToRead)
}

_offset += bytesRead.Length;
if (_offset == _totalLength)
return bytesRead;
}

protected override void Dispose(bool disposing)
{
if (_isDisposed)
{
Dispose(true);
return;
}
return bytesRead;
_streamCts?.Cancel();
_streamCts?.Dispose();
try
{
_ = _jsStreamReference?.DisposeAsync().Preserve();
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Preserve() extension method does not exist in the codebase. This will cause a compilation error.

To properly handle the fire-and-forget disposal in a synchronous context, you should use .AsTask() or simply discard the ValueTask without calling any method on it. Consider one of these alternatives:

// Option 1: Just discard the ValueTask (allows it to be collected)
_ = _jsStreamReference?.DisposeAsync();

or if you need to ensure the task runs:

// Option 2: Convert to Task (ensures the task runs)
_ = _jsStreamReference?.DisposeAsync().AsTask();
Suggested change
_ = _jsStreamReference?.DisposeAsync().Preserve();
_ = _jsStreamReference?.DisposeAsync();

Copilot uses AI. Check for mistakes.
}
catch
{
}

_isDisposed = true;

base.Dispose(disposing);
}
public override async ValueTask DisposeAsync()
{
if (_isDisposed)
{
return;
}

_streamCts?.Cancel();
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _streamCts CancellationTokenSource is never disposed, which can lead to resource leaks. You should dispose it in both Dispose and DisposeAsync methods.

Add disposal after cancellation:

_streamCts?.Cancel();
_streamCts?.Dispose();

Copilot uses AI. Check for mistakes.
_streamCts?.Dispose();

try
{
if (_jsStreamReference is not null)
{
await _jsStreamReference.DisposeAsync();
}
}
catch
{
}

_isDisposed = true;

await base.DisposeAsync();
}
}
23 changes: 22 additions & 1 deletion src/Components/WebAssembly/WebAssembly/test/PullFromJSDataStreamTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using Microsoft.AspNetCore.InternalTesting;
using Microsoft.JSInterop;
using Moq;
Expand Down Expand Up @@ -101,6 +100,28 @@ public async Task ReceiveData_JSProvidesExcessData_Throws2()
Assert.Equal("Failed to read the requested number of bytes from the stream.", ex.Message);
}

[Fact]
public void Dispose_CallsDisposeAsyncOnJSStreamReference()
{
var jsStreamReferenceMock = new Mock<IJSStreamReference>();
var stream = PullFromJSDataStream.CreateJSDataStream(_jsRuntime, jsStreamReferenceMock.Object, totalLength: 10, cancellationToken: CancellationToken.None);

stream.Dispose();

jsStreamReferenceMock.Verify(x => x.DisposeAsync(), Times.Once);
}

[Fact]
public async Task DisposeAsync_CallsDisposeAsyncOnJSStreamReference()
{
var jsStreamReferenceMock = new Mock<IJSStreamReference>();
var stream = PullFromJSDataStream.CreateJSDataStream(_jsRuntime, jsStreamReferenceMock.Object, totalLength: 10, cancellationToken: CancellationToken.None);

await stream.DisposeAsync();

jsStreamReferenceMock.Verify(x => x.DisposeAsync(), Times.Once);
}

private static PullFromJSDataStream CreateJSDataStream(byte[] data, IJSRuntime runtime = null)
{
runtime ??= new TestJSRuntime(data);
Expand Down
Loading