Repository navigation
cffi: create the DDict once per ZstdCompressionDict - #362
Open
pylaterreur wants to merge 1 commit into
Open
pylaterreur wants to merge 1 commit into
pylaterreur wants to merge 1 commit into
Conversation
ZstdCompressionDict._ddict was meant to create the DDict once and cache it in the instance __dict__, as the C backend does in ensure_ddict(). But a property takes precedence over the instance __dict__, so the cache was never read: every decompressor creation and operation created a new DDict and dropped the previous one, which freed it while another decompressor's context could still reference it. Decompressing then failed with "Dictionary mismatch" or crashed. Read the cached DDict before creating one, and create it under a lock so that concurrent first uses don't create two and free one of them. Closes indygreg#361. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GwUvhmzgfkiZ6vo4uGyesN
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
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.
Fixes #361. With the CFFI backend, using one
ZstdCompressionDictwith more than oneZstdDecompressorcan crash the interpreter or fail withDictionary mismatch.ZstdCompressionDict._ddictwas meant to create the DDict once and cache it in the instance__dict__, as the C backend does inensure_ddict(). Butpropertytakes precedence over the instance__dict__, so the cache was never read.ZSTD_DCtx_refDDict().The property now returns the cached DDict when there is one. It creates the DDict under a module-level lock, so two threads that use a dictionary for the first time at the same moment can't each create one and free the other's. The DDict now lives as long as the dictionary, as in the C backend.
test_dictionary_shared_by_decompressorscreates astream_reader(), then a second decompressor for the same dictionary, and checks both outputs. Without the fix, it crashes the test process with the CFFI backend on CPython 3.15.0 and 3.15.0t. With the fix, it passes on both backends.Local results on Linux x86_64 with GitHub's CPython builds, using test.yml's settings (uv 0.8.11,
ZSTD_SLOW_TESTS=1,ZSTD_WARNINGS_AS_ERRORS=1, hypothesisciprofile,PYTHONDEVMODE=1) with-n 2:Other checks:
check,format --check) and mypy, run as typing.yml does, report no problems.macos-26-intel, 3.13, ran into hypothesis's 20 s deadline intest_stream_source_readinto. That test is already flaky on main, and ci: fix the anaconda, wheel, external-zstd and flaky test workflows #358 removes the deadline.Prepared with the help of Claude Code.
🤖 Generated with Claude Code