Tewr / BlazorFileReader

Library for creating read-only file streams from file input elements or drop targets in Blazor.
MIT License
425 stars 61 forks source link

Deadlock on Stream.ReadAsync #165

Closed Banyc closed 4 years ago

Banyc commented 4 years ago

Describe the bug

ReadAsync will hang forever at the second call with the different offset and count.

To Reproduce Steps to reproduce the behavior:

To run the code below:

await using (Stream stream = await file.OpenReadAsync()) {
    int fileSize = (int)fileInfo.Size;
    int start = 0;
    int end = 0;
    do
    {
        end = start + bufferSize;
        if (end > fileSize)
        {
            end = fileSize;
        }

        await stream.ReadAsync(buffer, start, end - start);

        start += bufferSize;

    } while (end < fileSize);
}

Expected behavior

No deadlock or forever-hanging.

Screenshots

Unrelated.

Project type 'Server-side'/'SSB' or 'Client-side'/'CSB' (or when applicable, specify 'Both SSB/CSB')

Environment

Additional context

Chunk uploading.

Banyc commented 4 years ago

It seems that the offset argument could not be more than 0.

Tewr commented 4 years ago

@Banyc hello, thank you for the bug report.

This is a regression from the previous version I assume?

Tewr commented 4 years ago

Ok so I have a just few minutes to test this tonight. First of all, your code is incorrect -- that doesn't mean there is a not a bug though.

how is your buffer declared?

I declared it var buffer = new byte[BufferSize]; and this gives me an error, as we are reading outside of the bounds

I declared it var buffer = new byte[fileInfo.Size]; and no deadlock....

Banyc commented 4 years ago

The buffer size is assigned with a fixed length:

private static int bufferSize = 1024;

The full razor component could be found here:

https://gist.github.com/Banyc/c382440c71301f1f8aa38edc70de0ab9

The working edition could be found here:

https://github.com/Banyc/Sharer/blob/5ce5cf7725c59629e82f957b66675c1202e224bf/src/SharerBlazorServer/Pages/Upload.razor

Tewr commented 4 years ago

Ok so If I understand correcty you are trying to run the following code:

                var bufferSize = 1024;
                var buffer = new byte[bufferSize];
                await using (var stream = await file.OpenReadAsync())
                {
                    int fileSize = (int)fileInfo.Size;
                    int start = 0;
                    int end = 0;
                    do
                    {
                        end = start + bufferSize;
                        if (end > fileSize)
                        {
                            end = fileSize;
                        }

                        await stream.ReadAsync(buffer, start, end - start);

                        start += bufferSize;

                    } while (end < fileSize);
                }

If you wrap it in a try-catch you will see the following exception when start(offset) = 1024 and end-start(count)=1024: System.ArgumentException: 'Destination array was not long enough. Check the destination index, length, and the array's lower bounds. '

This is correct, as the offset parameter is a reference to the buffer you are passing. Your buffer can be adressed from 0 to 1024, and you are trying to write to it from offset 1024, that is, outside its bounds.

Try to read up on the examples in the demo project or at microsoft examples on how to read a stream.

I think that what you actually "want" is to call await stream.ReadAsync(buffer, offset=0, end - start);, this is what would make sense. The code path you are running, at least with this buffer size, is the same as the previous version of the library.

Using an offset parameter for any other value than 0 is only for cases when your buffer.Length is larger than the buffer size you are trying to read from the stream, or if you want to write the data out of order for some reason. Your code could make sense if you would re-use the same buffer and that you initialize the buffer to a length of fileInfo.Size.

Does this make sense?

Banyc commented 4 years ago

Huge thanks! It was me calling the method in the wrong way XD