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

VS 16.2.0 regression: CopyToAsync from DeflateStream to GzipStream throws NotImplementedException #16122

Closed
steveisok opened this issue Aug 8, 2019 · 4 comments · Fixed by #16523 or xamarin/xamarin-android#3578

Comments

@steveisok
Copy link
Contributor

@steveisok steveisok commented Aug 8, 2019

Copied from xamarin/xamarin-android#3397

Steps to Reproduce

  1. Use the ZipArchive and ZipArchiveEntry classes to open a stream to an entry in a zip file
  2. Set up a GZipStream compressing to a MemoryStream
  3. Copy the zip entry stream to the GZipStream
  4. Observe the NotImplementedException

Repro project: https://github.com/mfeingol/repros/tree/master/Xamarin.Android-3397-CopyToAsync

Expected Behavior

Stream copy is successful, as it was in prior versions of the Mono runtime.

Actual Behavior

NotImplementedException:

  at System.IO.Compression.DeflateStream.WriteAsyncMemory (System.ReadOnlyMemory`1[T] source, System.Threading.CancellationToken cancellationToken) [0x00000] in <ca7419b40e504a6dbe088f6fe95d09aa>:0 
  at (wrapper remoting-invoke-with-check) System.IO.Compression.DeflateStream.WriteAsyncMemory(System.ReadOnlyMemory`1<byte>,System.Threading.CancellationToken)
  at System.IO.Compression.GZipStream.WriteAsync (System.ReadOnlyMemory`1[T] buffer, System.Threading.CancellationToken cancellationToken) [0x00026] in <ca7419b40e504a6dbe088f6fe95d09aa>:0 
  at System.IO.Stream.CopyToAsyncInternal (System.IO.Stream destination, System.Int32 bufferSize, System.Threading.CancellationToken cancellationToken) [0x000c7] in <ff07eae8184a40a08e79049bbcb31a0e>:0 
  at CopyToAsync.MainPage.Repro (System.Object sender, System.EventArgs args) [0x0008a] in C:\Users\mfeingol\source\Repos\CopyToAsync\CopyToAsync\CopyToAsync\MainPage.xaml.cs:35 

Example code:

ZipArchive archive = new ZipArchive(ZipFile, ZipArchiveMode.Read, true, Encoding.UTF8);
ZipArchiveEntry entry = archive.GetEntry("a4.jpg");
Stream src = entry.Open();

MemoryStream dest = new MemoryStream();

using (GZipStream gzip = new GZipStream(dest, CompressionLevel.Optimal, true))
    await src.CopyToAsync(gzip);

byte[] compressed = dest.ToArray();

Version Information

VS 16.2.0, fresh project from new project wizard, latest Xamarin.Forms Nuget packages (not that it's relevant)

Log File

log.txt

@steveisok

This comment has been minimized.

Copy link
Contributor Author

@steveisok steveisok commented Aug 8, 2019

Apparently VS 16.1.5 works, which is 2018-08.

@steveisok steveisok added this to the 2019-06 (6.4.xx) milestone Aug 8, 2019
@steveisok

This comment has been minimized.

Copy link
Contributor Author

@steveisok steveisok commented Aug 11, 2019

So, there is a slight difference that changes the code path to call DeflateStream.WriteAsyncMemory.

In 5.18 (2018-08), Mono was using the referencesource version of Stream, which opened its own buffer and called this WriteAsync method in GZipStream:

https://github.com/mono/corefx/blob/c976905c1de06786292a6fb01656b43650f262c9/src/System.IO.Compression/src/System/IO/Compression/GZipStream.cs#L177-L181

In 6.0 (2019-02), we changed changed to the corefx one via this commit:

46a2830

And now, that code rents a shared buffer and calls this method in GZipStream:

https://github.com/mono/corefx/blob/c976905c1de06786292a6fb01656b43650f262c9/src/System.IO.Compression/src/System/IO/Compression/GZipStream.cs#L183-L197

This is a regression. To fix, I'm not sure if there's anything stopping us from taking the corefx version, but I'm sure we can at the very least take the WriteAsyncMemory piece.

@steveisok

This comment has been minimized.

Copy link
Contributor Author

@steveisok steveisok commented Aug 11, 2019

@marek-safar Should this go back to 2019-02? Since that's technically when it started to regress.

@marek-safar

This comment has been minimized.

Copy link
Member

@marek-safar marek-safar commented Aug 12, 2019

@steveisok we can decide when we have a fix

steveisok pushed a commit to steveisok/mono that referenced this issue Aug 27, 2019
Since we put in the corefx version of GZipStream, it is expected that
our version of DeflateStream actually implements Write+ReadAsyncMemory.

This change does that by just calling the plain WriteAsync & ReadAsync
methods until we decide (or not) to pull in the corefx DeflateStream.
steveisok added a commit that referenced this issue Aug 28, 2019
Resolves #16122

Since we put in the corefx version of GZipStream, it is expected that
our version of DeflateStream actually implements Write+ReadAsyncMemory.

This change does that by just calling the plain WriteAsync & ReadAsync
methods until we decide (or not) to pull in the corefx DeflateStream.
monojenkins added a commit to monojenkins/mono that referenced this issue Aug 28, 2019
Since we put in the corefx version of GZipStream, it is expected that
our version of DeflateStream actually implements Write+ReadAsyncMemory.

This change does that by just calling the plain WriteAsync & ReadAsync
methods until we decide (or not) to pull in the corefx DeflateStream.
monojenkins added a commit to monojenkins/mono that referenced this issue Aug 28, 2019
Since we put in the corefx version of GZipStream, it is expected that
our version of DeflateStream actually implements Write+ReadAsyncMemory.

This change does that by just calling the plain WriteAsync & ReadAsync
methods until we decide (or not) to pull in the corefx DeflateStream.
monojenkins added a commit to monojenkins/mono that referenced this issue Aug 28, 2019
Since we put in the corefx version of GZipStream, it is expected that
our version of DeflateStream actually implements Write+ReadAsyncMemory.

This change does that by just calling the plain WriteAsync & ReadAsync
methods until we decide (or not) to pull in the corefx DeflateStream.
akoeplinger added a commit that referenced this issue Aug 29, 2019
…16551)

Resolves #16122

Since we put in the corefx version of GZipStream, it is expected that
our version of DeflateStream actually implements Write+ReadAsyncMemory.

This change does that by just calling the plain WriteAsync & ReadAsync
methods until we decide (or not) to pull in the corefx DeflateStream.

Backport of #16523.
steveisok added a commit that referenced this issue Aug 29, 2019
Resolves #16122

Since we put in the corefx version of GZipStream, it is expected that
our version of DeflateStream actually implements Write+ReadAsyncMemory.

This change does that by just calling the plain WriteAsync & ReadAsync
methods until we decide (or not) to pull in the corefx DeflateStream.

Backport of #16523.
akoeplinger added a commit that referenced this issue Aug 29, 2019
…16549)

Resolves #16122

Since we put in the corefx version of GZipStream, it is expected that
our version of DeflateStream actually implements Write+ReadAsyncMemory.

This change does that by just calling the plain WriteAsync & ReadAsync
methods until we decide (or not) to pull in the corefx DeflateStream.

Backport of #16523.
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Sep 3, 2019
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Sep 4, 2019
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Sep 4, 2019
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Sep 4, 2019
Changes: mono/mono@e6f5369...6256b82

Upstream-Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/976811
Upstream-Fixes: mono/mono#15751
Upstream-Fixes: mono/mono#15825
Upstream-Fixes: mono/mono#16032
Upstream-Fixes: mono/mono#16122
Upstream-Fixes: mono/mono#16486
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Sep 4, 2019
Changes: mono/mono@e6f5369...6256b82

Upstream-Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/976811
Upstream-Fixes: mono/mono#15751
Upstream-Fixes: mono/mono#15825
Upstream-Fixes: mono/mono#16032
Upstream-Fixes: mono/mono#16122
Upstream-Fixes: mono/mono#16486
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Sep 4, 2019
jonpryor added a commit to xamarin/xamarin-android that referenced this issue Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.