Blazor DotNetStreamReference Race condition when arrayBuffer is not awaited/read in javascript with using keyword.

[Tyme-Bleyaert]

Is there an existing issue for this?

• [X] I have searched the existing issues

Describe the bug

There is this weird bug I encountered with Blazor Server (PreRendered) in .NET 6.

When using code like this:

private async Task DrawImage()
{
    var fileInfo = new FileInfo(_pathToImage);
    var module = await _moduleRef.Value;
    var doesImageExist = await module.InvokeAsync<bool>("doesImageExist", fileInfo.Name);

    var fileStream = new FileStream(_pathToImage, FileMode.Open, FileAccess.Read);
    using var streamRef = new DotNetStreamReference(fileStream);
    await module.InvokeVoidAsync("createImageAndAddToLayer", _layerRef, streamRef, fileInfo.Name, _contentType, x, y);
}

With this Javascript:

export async function createImageAndAddToLayer(layerRef, imageStream, imgName, contentType, x, y) {
    var image;

    if ((imageDict[imgName] === null || imageDict[imgName] === undefined) &&
        (imageStream !== null && imageStream !== undefined)) {

        //Get arrayBuffer from image stream
        const arrayBuffer = await imageStream.arrayBuffer(); //Issue here when condition is false
        const blob = new Blob([arrayBuffer], { type: contentType });

        //Create image and object url
        var imageUrl = URL.createObjectURL(blob);
        image = new Image();
        image.src = imageUrl;
        image.onload = () => {
            URL.revokeObjectURL(imageUrl);
        }
        //Save image in dictionary
        imageDict[imgName] = image;
    } else {
        //Load existing image from dictionary
        image = imageDict[imgName]; 
    }

    // Create Konva image to render on canvas
    var konvaImage = new Konva.Image({
        x: x,
        y: y,
    });
    konvaImage.image(image);

    layerRef.add(konvaImage);
}

This always results in an unhandled error you cannot catch in Blazor anywhere. The error only occurs when the arrayBuffer is not awaited in javascript (so when DrawImage is called the second time for the same image).

 Error: System.ObjectDisposedException: Cannot access a closed file.
   at System.IO.FileStream.ReadAsync(Memory`1 buffer, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.Components.Server.Circuits.CircuitHost.<>c__DisplayClass46_0.<<SendDotNetStreamAsync>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Components.Rendering.RendererSynchronizationContext.<>c__11`1.<<InvokeAsync>b__11_0>d.MoveNext()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Components.Server.Circuits.CircuitHost.SendDotNetStreamAsync(DotNetStreamReference dotNetStreamReference, Int64 streamId, Byte[] buffer)

I can fix this by removing the using from using var streamRef = new DotNetStreamReference(fileStream);. Or moving const arrayBuffer = await imageStream.arrayBuffer(); out of the if statement.

What can cause this and why are we unable to catch the exception?

Expected Behavior

No error or an exception we can actually catch.

Steps To Reproduce

Just let the DrawImage execute twice for the same image.

If needed, i can create a minimal reproduceable repository.

Exceptions (if any)

No response

.NET Version

8.0.100

Anything else?

No response

[ghost]

Thank you for filing this issue. In order for us to investigate this issue, please provide a minimal repro project that illustrates the problem without unnecessary code. Please share with us in a public GitHub repo because we cannot open ZIP attachments, and don't include any confidential content.

[Tyme-Bleyaert]

Created repo here : https://github.com/Tyme-Bleyaert/BlazorDotNetStreamReferenceIssue

When you run the repo, and set a breakpoint in the catch you will see it never breaks there (unless it's because the circuit already disconnected). So we get an error, while the arrayBuffer is never read and we cannot even catch it.

[Tyme-Bleyaert]

Update: this also happens in .Net 8

Edit: I think I know what happens :

The cause of this issue is probably a race condition. When we look at : https://github.com/dotnet/aspnetcore/blob/afb5f4dfc3d0ad59d0b2d0ed82a2faa2c7dc5487/src/Components/Server/src/ComponentHub.cs#L268

The stream is being send to JavaScript, and since we don't read the stream in JavaScript and the stream is already closed. The while loop still thinks https://github.com/dotnet/aspnetcore/blob/afb5f4dfc3d0ad59d0b2d0ed82a2faa2c7dc5487/src/Components/Server/src/ComponentHub.cs#L287 This has no clue that the stream is already closed / disposed and tries to read anyway. This throws the exception in https://github.com/dotnet/aspnetcore/blob/afb5f4dfc3d0ad59d0b2d0ed82a2faa2c7dc5487/src/Components/Server/src/Circuits/CircuitHost.cs#L475 And this exception we cannot catch.

I wonder if we can't check if the stream is still open like this:

while (dotNetStreamReference.Stream.CanRead && (bytesRead = await circuitHost.SendDotNetStreamAsync(dotNetStreamReference, streamId, buffer)) > 0)
{
    yield return new ArraySegment<byte>(buffer, 0, bytesRead);
}

But I guess it's still a race condition if the stream gets disposed right after the Stream.CanRead check.

That why I don't get the issue when not using using or when I await the buffer in JavaScript.

When using a conditional stream usage in JS we should never use the using keyword or either do the conditional check to JS before sending the stream to JS.

[MackinnonBuck]

The reason this happens is due to the following combination:

1. SignalR requires streaming to be initiated from the client, so the server sends a message to the client with a stream ID, then the client sends another message back to the server requesting that stream.
2. The JS interop call providing the stream to JS doesn't block on receiving the stream message from the client.

This creates a race condition, as you correctly pointed out. If the invoked JS method doesn't do anything with the .NET stream, the server may receive the message to start reading the stream after execution returned back to the .NET method disposing the stream. This causes the ObjectDisposedException you're seeing.

One way to address this might be catching that exception and treating reading a disposed stream similar to the EOF case.

[Tyme-Bleyaert]

@MackinnonBuck

Since the exception is already handled in the CircuitHost, how do you want to handle it in the ComponentHub ?

[lofcz]

@MackinnonBuck could you please elaborate on how to handle the ObjectDisposedException in the scenario @Tyme-Bleyaert presented? We are facing the same issue.

[MackinnonBuck]

could you please elaborate on how to handle the ObjectDisposedException...

@lofcz Apologies if I wasn't clear - my suggestion was to handle it in the framework (but on second thought, it would just be better to avoid throwing that exception anyway). AFAICT, there's not currently a way to handle this exception in application code.

The best thing apps could do (and should) would be not sending the DotNetStreamReference over a JS interop call unless it's definitely going to get used in JS. If you can't determine purely in .NET code whether the stream would be required by JS logic, you could even split up the JS interop call into two:

• The first JS interop call optimistically runs the JS function without providing a stream reference
• A second JS interop call happens only if the return value from first one indicated that a stream reference is required

Note that if a DotNetStreamReference gets passed as an argument to a JS interop call, even if it doesn't get directly used by the invoked JS function, the stream still gets read in .NET and sent over the network.

Also, it's also not really necessary to manually dispose the DotNetStreamReference (either with using or by directly calling .Dispose()), because the stream will get disposed automatically after being read and transmitted to the browser.

---

All that said, it still seems like a framework bug to kill the circuit if the stream is disposed during transmission.

I see a few potential options for a fix:

1. The simplest option: if the stream gets disposed during transmission, pretend that the stream was read to completion. This is likely not ideal because the browser has no way to distinguish the error case from the happy path.
2. Send an error message to the browser and throw an error at the JS call site reading the stream. If the stream does not get read directly in JS, then the error message doesn't surface. This is probably the right thing to do.
3. Throw an exception in .NET when disposing the stream reference during transmission. It's hard to say whether this would be a breaking change, considering that the current behavior kills the circuit, but it seems weird that you wouldn't be able to do this.

So, my proposal for a fix would be option 2.

[MackinnonBuck]

Update: We've decided not to change this area and instead we're going to update our docs to recommend patterns that avoid disposing DotNetStreamReferences during transmission to the browser.

@guardrex, do you think we could add some content to this section of our docs covering these details?

We should also update that example to not utilize a using statement because that will actually dispose the stream twice (once when transmission completes and again when streamRef exits the scope it was declared in). Note that you can still use using if either of the following conditions are met:

• leaveOpen is set to true and .NET code waits for the stream to get completely consumed by JS before exiting the current scope
  • e.g., it's valid to set leaveOpen to true and await a JS method invocation that returns a promise that completes only after the stream is completely consumed.
• The DotNetStreamReference never gets passed to JS via JS interop

But in most cases, leaveOpen would be false (it's the default), and the DotNetStreamReference would get passed to JS, so you should generally not manually dispose the DotNetObjectReference. You only have to worry about manually disposing the stream if leaveOpen is true or the stream reference is never passed to JS.

[MackinnonBuck]

Closing since the docs work for this is now complete.