Skip to content

Reliable clean-up of SafeHandles in Deflater/Inflater #114826

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

edwardneal
Copy link
Contributor

This reimplements #71991 in Deflater and Inflater. The PR was previously reverted in #85001 as a result of issue #84994.

As noted in the reverting PR, Deflater simply needed to dispose of the returned SafeHandle and supress its own finalizer if CreateZLibStreamForDeflate threw an exception. We now do this.

We couldn't do this for Inflater at the time - it had to support concatenated GZip data, and as part of that support it would recreate its ZLibStreamHandle. As a result of #113587, this recreation no longer happens and we can treat Inflater as we would treat Deflater. I've refactored Inflater.InflateInit out of existence and made _zlibStream readonly to reflect this.

This also incorporates a change to both class' DeallocateBufferHandle methods (when called by Dispose) to prevent them dereferencing a ZLibStreamHandle after it's been disposed of. This is roughly how #84994 was originally discovered.

If the call to CreateZLibStreamForXXflate throws an exception from the constructor, explicitly dispose of the ZLibStreamHandle (rather than relying on the finalizer to do it.)

Also incorporates a change to DeallocateBufferHandle (called from the Dispose path) to prevent it using a ZlibStreamHandle after it's been Disposed.
This aligns Inflater with Deflater.
Removed now-unnecessary usings.
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 18, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-io-compression
See info in area-owners.md if you want to be subscribed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.IO.Compression community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants