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

Fix Write_DataReadFromDesiredOffset test #45477

Merged
1 commit merged into from
Dec 2, 2020
Merged

Conversation

stephentoub
Copy link
Member

(Already failing on my goal not to submit PRs while on vacation, but I wanted to help squash this test failure taking out multiple PRs, and a "blocking-clean-ci" issue assigned to me wasn't helping with relaxation ;-)

Sporadic null reference exceptions were occurring in CI as part of GZipStream.Dispose. The call to writeable.Dispose in the writing task can race with the invisible writeable.Dispose that happens as part of the disposal at the end of the block, due to the earlier using statement. If the writeable.Dispose in the task nulls out the _deflateStream inside of the GZipStream after the implicit Dispose has checked for null but before it calls Dispose on the field, it'll null ref. The fix is simple and something that should have been there anyway: explicitly wait for the writing task to complete, which guarantees the implicit disposal won't race with the explicit one.

Fixes #44173

cc: @safern, @geoffkizer

(Already failing on my goal not to submit PRs while on vacation, but I wanted to help squash this test failure taking out multiple PRs.)

Sporadic null reference exceptions were occurring in CI as part of GZipStream.Dispose.  The call to writeable.Dispose in the writing task can race with the invisible writeable.Dispose that happens as part of the disposal at the end of the block, due to the earlier using statement.  If the writeable.Dispose in the task nulls out the _deflateStream inside of the GZipStream after the implicit Dispose has checked for null but before it calls Dispose on the field, it'll null ref.  The fix is simple and something that should have been there anyway: explicitly wait for the writing task to complete, which guarantees the implicit disposal won't race with the explicit one.
@stephentoub stephentoub added this to the 6.0.0 milestone Dec 2, 2020
@safern
Copy link
Member

safern commented Dec 2, 2020

Already failing on my goal not to submit PRs while on vacation, but I wanted to help squash this test failure taking out multiple PRs, and a "blocking-clean-ci" issue assigned to me wasn't helping with relaxation ;-)

Thanks for fixing. I didn't know you were on vacation 😄 -- enjoy it and relax!

@ghost
Copy link

ghost commented Dec 2, 2020

Hello @safern!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit b9de3d0 into dotnet:master Dec 2, 2020
@stephentoub stephentoub deleted the fixgzip branch December 2, 2020 19:34
@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.IO.Compression.GzipStreamUnitTests.Write_DataReadFromDesiredOffset failed
2 participants