Skip to content

cffi: create the DDict once per ZstdCompressionDict - #362

Open
pylaterreur wants to merge 1 commit into
indygreg:mainfrom
pylaterreur:cffi-shared-ddict
Open

pylaterreur wants to merge 1 commit into
indygreg:mainfrom
pylaterreur:cffi-shared-ddict

Conversation

@pylaterreur

Copy link
Copy Markdown

Fixes #361. With the CFFI backend, using one ZstdCompressionDict with more than one ZstdDecompressor can crash the interpreter or fail with Dictionary mismatch.

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 property takes precedence over the instance __dict__, so the cache was never read.

  • Every decompressor creation and every decompression created a new DDict and dropped the previous one.
  • Dropping it freed it while another decompressor's context could still reference it through 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_decompressors creates a stream_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, hypothesis ci profile, PYTHONDEVMODE=1) with -n 2:

C CFFI
3.15.0 293 passed 267 passed, 26 skipped
3.15.0t 293 passed 267 passed, 26 skipped

Other checks:

  • A stress test where 8 threads each create their own decompressors for one shared dictionary segfaulted every time before the fix. After the fix it passes 3 runs out of 3 (1600/1600 round trips) on both builds.
  • ruff 0.17.1 (check, format --check) and mypy, run as typing.yml does, report no problems.
  • My fork's CI: in test.yml, the new test passes in all 48 jobs (Linux, macOS, Windows; 3.10 to 3.14t; every backend step). In pypy.yml, it passes in all 7 test jobs, which use the CFFI backend. The one failed job, macos-26-intel, 3.13, ran into hypothesis's 20 s deadline in test_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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CFFI backend: crash when ZstdDecompressor instances share a ZstdCompressionDict

1 participant