[BUG] BlockBlobWriteStream does not implement IAsyncDisposable

klesta490 commented 1 year ago

Library name and version

Azure.Storage.Blobs 12.13.1.0

Describe the bug

When using

await using (var stream = await blockBlob.OpenWriteAsync(true))

await stream.WriteAsync(new byte[4]);

Because of missing DisposeAsync implementation in BlockBlobWriteStream, generic implementation is called on Stream base class. This implementaiton calls sync Dispose() witch consequently calls Flush() - synchronous https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/storage/Azure.Storage.Common/src/Shared/StorageWriteStream.cs#L227

 public override void Flush()
            => FlushInternal(
                async: false,
                cancellationToken: default).EnsureCompleted();

and in this FlushInternal there is StagingBlock if needed. And then block ids are always commited. - synchronous https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/storage/Azure.Storage.Blobs/src/BlockBlobWriteStream.cs#L87

There is no way to avoid it, if I manually call FlushAsync() before DisposeAsync(), then in FlushInternal no blocks are staged, but I cannot avoid synchronous call for commiting block ids.

Am I missing something?

Expected behavior

DisposeAsync is properly implemented, so there is no sychronous call when using async API.

Actual behavior

DisposeAsync is not implemented/overriden, so when DisposeAsync is called, sychronous call from Stream base class to store block ids is done.

Reproduction Steps

await using (var stream = await blockBlob.OpenWriteAsync(true))

await stream.WriteAsync(new byte[4]);

Environment

No response

github-actions[bot] commented 1 year ago

Thank you for your feedback. This has been routed to the support team for assistance.

klesta490 commented 1 year ago

Any update?

navba-MSFT commented 1 year ago

Adding Service team to look into this.

github-actions[bot] commented 1 year ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

klesta490 commented 11 months ago

Hello, any update? Is anyone there? @jsquire

jsquire commented 11 months ago

@klesta490: I apologize that this has not been followed-up on. Unfortunately, I do not have direct insight here. We'll need the folks that own the Storage library to offer thoughts.

//cc: @seanmcc-msft, @amnguye //fyi: @schaabs

mkopsa-quadient commented 4 months ago

Hello, is there any progress? @jsquire

jsquire commented 4 months ago

@mkopsa-quadient : The answer is the same as the one right above your comment. This is not an issue that I will have insight on and requires the owners of the issue to offer thoughts.

mkopsa-quadient commented 4 months ago

OK. Can @seanmcc-msft or @amnguye answer?

Azure / azure-sdk-for-net