Skip to content
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.

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

feature request: full async/await pattern stream support #223

Open
coelacanth16 opened this issue Apr 11, 2018 · 6 comments · Fixed by #574
Open

feature request: full async/await pattern stream support #223

coelacanth16 opened this issue Apr 11, 2018 · 6 comments · Fixed by #574
Labels
enhancement Feature request or other improvements of existing functionality

Comments

@coelacanth16
Copy link

Expected behavior

I could not find any related topics, except an unanswered issue "CopyToAsync fails with ZipOutputStream #167".

One of c#'s greatest features over other languages is the async/await pattern. The .net streams all support async/await since ver 4.5 and there could be major implications and performance enhancers for FileStream in particular. Is there any plan to implement this support in this library?

@piksel piksel added the enhancement Feature request or other improvements of existing functionality label May 12, 2018
@Numpsy
Copy link
Contributor

Numpsy commented Apr 5, 2020

So, I had a little bit of a look at this (partly as it's requested, partly out of thinking it might be interesting to tinker with). I initially wondered if it would be possible to start at the bottom of the stream and add it a bit at a time, but the way the zip/gzip streams are imple,ented as subclasses of the deflator streams rather than as containers causes issues with that (if you only implement the async methds of the base class, async calls on the subclasses go through to those and miss the archive specific pieces).
So, bit more of an all-in change there.

However, as a starter - I adapted a few on the unit tests to use async stream calls as a test, and those changes could be made independantly of library changes (as it stands and async calls will be forwarded to the sync versions by the base stream class, but it async methods were to be implemented later that would mean there was already a test base for them).

@piksel and comments and/or objections to the idea of adding some async tests to the existing zip test suite? (would just be variants of the existing tests but with async stream calls to start with)

@piksel
Copy link
Member

piksel commented Apr 7, 2020

No, I guess that would be fine.

@piksel piksel added this to Preparation in Feature Requests Nov 22, 2020
@piksel piksel self-assigned this Nov 22, 2020
This was unlinked from pull requests Nov 22, 2020
@piksel piksel moved this from Preparation to In progress in Feature Requests Nov 22, 2020
@piksel piksel moved this from In progress to Ready to implement in Feature Requests Nov 22, 2020
@piksel piksel removed their assignment Nov 22, 2020
0xced added a commit to 0xced/SharpZipLib that referenced this issue Dec 7, 2020
This is a very rough first pass at adding async support to ZipOutputStream.

All method doing synchronous stream writes have been duplicated with an asynchronous counterpart, starting from `ZipOutputStream.PutNextEntryAsync()`. Unfortunately, this makes a lot of duplicated code but is unavoidable to have both sync + async code paths.

.NET Standard 2.1 has been added to the target frameworks. It's required to get full async support with `Stream.DisposeAsync()`.

New unit tests must be written to cover the new async code paths.

Fixes icsharpcode#223
@mark-at-tusksoft
Copy link

Running into the same issue as reported in a similar library
haf/DotNetZip.Semverd#60

 Uncaught (in promise) Error: System.NotSupportedException: Synchronous reads are not supported.
   at Microsoft.AspNetCore.Components.Forms.BrowserFileStream.Read(Byte[] buffer, Int32 offset, Int32 count)
   at ICSharpCode.SharpZipLib.Zip.Compression.Streams.InflaterInputBuffer.Fill()
   at ICSharpCode.SharpZipLib.Zip.Compression.Streams.InflaterInputBuffer.ReadLeByte()
   at ICSharpCode.SharpZipLib.Zip.Compression.Streams.InflaterInputBuffer.ReadLeShort()
   at ICSharpCode.SharpZipLib.Zip.Compression.Streams.InflaterInputBuffer.ReadLeInt()
   at ICSharpCode.SharpZipLib.Zip.ZipInputStream.GetNextEntry()

@Numpsy
Copy link
Contributor

Numpsy commented May 6, 2021

It's being worked on a bit at a time, but there's quite a bit of work to change everywhere.

@piksel piksel linked a pull request May 8, 2021 that will close this issue
Feature Requests automation moved this from Ready to implement to Done Oct 9, 2021
@Numpsy
Copy link
Contributor

Numpsy commented Oct 9, 2021

Do we want to close this, or leave it in case of adding async support to the other stream classes (Bzip2 etc) ?

@piksel
Copy link
Member

piksel commented Oct 9, 2021

They should probably be their own issues, although much of the work done in #574 can be used to implement them in the other streams as well. full async/await pattern stream is kind of vague as a issue scope. I will leave it open for now...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request or other improvements of existing functionality
Projects
4 participants