Add async dispose support to ZipArchive

[MarcusWichelmann]

In my application I am creating a ZipArchive that writes directly to the ASP.Net Core body stream, like that:

using var zipArchive = new ZipArchive(Response.Body, ZipArchiveMode.Create, true);

Asynchronously writing file contents to that archive works fine, but as soon as the writing is finished and the ZipArchive gets disposed, the ZipArchive tries to synchronously write some more data to the stream. This results in an exception, because Kestrel, by default, doesn't allow synchronous writes anymore:

System.InvalidOperationException: Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead.
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpResponseStream.Write(Byte[] buffer, Int32 offset, Int32 count)
   at Microsoft.AspNetCore.ResponseCompression.ResponseCompressionBody.Write(Byte[] buffer, Int32 offset, Int32 count)
   at System.IO.Compression.PositionPreservingWriteOnlyStreamWrapper.Write(Byte[] buffer, Int32 offset, Int32 count)
   at System.IO.BinaryWriter.Write(UInt32 value)
   at System.IO.Compression.ZipArchiveEntry.WriteCentralDirectoryFileHeader()
   at System.IO.Compression.ZipArchive.WriteFile()
   at System.IO.Compression.ZipArchive.Dispose(Boolean disposing)
   at System.IO.Compression.ZipArchive.Dispose()

Enabling async disposal for this class and then doing the necessary writes asynchronously would probably resolve this issue.

[vtchalkov]

looks like duplicate of https://github.com/dotnet/corefx/issues/5681

[davidfowl]

Does it work if you use FlushAsync before disposing? There is no flush API...

[carlossanlop]

Triage:
@vtchalkov the ZipFile issue is not a duplicate.
This looks like a legitimate scenario. Next step would be to add a PR to implement it and add DisposeAsync.

[manandre]

This needs first to add async methods to BinaryWriter via dotnet/corefx#8382

[manandre]

Test running successfully with manandre#1 👍

[Fact]
public static async Task DiposeAsyncCallsWriteAsyncOnly()
{
    MemoryStream ms = new MemoryStream();
    CallTrackingStream trackingStream = new CallTrackingStream(ms);

    await using (ZipArchive archive = new ZipArchive(trackingStream, ZipArchiveMode.Create))
    {
        archive.CreateEntry("hey");
    }

    Assert.Equal(0, trackingStream.TimesCalled("Write"));
    Assert.NotEqual(0, trackingStream.TimesCalled("WriteAsync"));
}

[madelson]

This would be beneficial for us as well!

[anranruye]

Is there any progress?

[adamsitnik]

Is there any progress?

We haven't made any progress recently. I am going to move it to 7.0 milestone to make sure we discuss including it in .NET 7.0

[anranruye]

A workaround for this scenario is to use Response.BodyWriter.AsStream() instead.

[carlossanlop]

The discussion in this issue has been very helpful. I think we should implement all ZipFile and ZipArchive async APIs at the same time. For that reason, I'll close this issue in favor of the oldest and most generic issue (#1541), and will add a comment there to make sure to come back here to look at the comments here.