Repository navigation
fix: wire NativePointerWithRefCount into ref'd compress/decompress dictionaries - #119
Merged
Merged
Conversation
…ctionaries ZstdCompressContext#refDictionary and ZstdDecompressContext#refDictionary attached a raw pointer to a borrowed dictionary; closing that dictionary while a context still referenced it freed the pointer out from under the context, corrupting subsequent compress/decompress calls. Both dictionary classes now extend NativePointerWithRefCount, and both contexts retain/release across ref/replace/clear/reset/close so the native dictionary is only freed once every borrower has let go. This also closes two narrower gaps in that same bookkeeping: refDictionary() on an already-closed context used to leak a retain instead of failing cleanly, and loadDictionary()/refPrefix() natively supersede a previously ref'd CDict/DDict (per zstd.h) but weren't releasing the Java-side retain, delaying when memory could be freed. docs/how-to.md is updated to match — it previously described refDictionary as a plain borrow requiring the dictionary to outlive the context. New RefDictionaryTest covers closing a still-referenced dictionary, replacing one, clearing with null, reset/loadDictionary/refPrefix dropping the reference, refDictionary on a closed context, and the chaining contract. Verified against the pre-fix code in a scratch worktree: the core scenario crashes the JVM (Abort trap: 6, native heap-corruption abort from the use-after-free); the two narrower-gap tests fail with plain assertion errors (delayed-free, not a dangling pointer). All pass here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dfa1
force-pushed
the
fix/refdictionary-use-after-free
branch
from
September 11, 2026 20:05
44b68f3 to
8c8a875
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ZstdCompressContext#refDictionary/ZstdDecompressContext#refDictionaryattached a raw native pointer to a borrowedZstdCompressDictionary/ZstdDecompressDictionary. Closing that dictionary while a context still referenced it freed the pointer out from under the context — a use-after-free on every subsequent compress/decompress call.NativePointerWithRefCount(added in a prior commit but not yet wired in) instead ofNativeObject.retain()the dictionary onrefDictionary(...),release()the previous one on replace/clear, release on a parameterreset(...), and release intryClose()— so the native dictionary is only freed once every borrower, including the dictionary's own constructor reference, has let go.refDictionary()on an already-closed context leaked a retain instead of failing cleanly, (2)loadDictionary()/refPrefix()natively supersede a previously ref'd CDict/DDict (per zstd.h) but weren't releasing the Java-side retain, delaying when memory could be freed. Also fixed stale docs indocs/how-to.md.Test plan
RefDictionaryTest: closing a still-referenced compress/decompress dictionary, replacing a referenced dictionary, clearing withnull,resetdropping the reference,loadDictionary/refPrefixsuperseding the reference, refDictionary-on-a-closed-context, and the chaining contract.RefDictionaryTest.javaagainst the pre-fix commit (047958c) in a scratch worktree — it crashes the JVM (Abort trap: 6, exit 134, native heap-corruption abort from the use-after-free). Ran the follow-up tests against the first fix commit (43c0364) — 6 of them fail as expected (assertion failures, not crashes, since that gap was a delayed-free, not a dangling pointer). All pass on the final commit../mvnw -pl zstd validate(checkstyle) — clean./mvnw -pl zstd javadoc:javadoc— clean./mvnw -pl zstd test— full suite passes🤖 Generated with Claude Code