From c41a644b704ecc6f3c62558ffe94ea631361a760 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Le Borgne Date: Sat, 17 Oct 2026 00:14:14 +0100 Subject: [PATCH] cffi: create the DDict once per ZstdCompressionDict 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 #361. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GwUvhmzgfkiZ6vo4uGyesN --- docs/news.rst | 9 ++++++ tests/test_decompressor_decompress.py | 29 +++++++++++++++++ zstandard/backend_cffi.py | 45 +++++++++++++++++++-------- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 83641a75..b4e2f5e9 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -17,6 +17,15 @@ Backwards Compatibility Notes * cibuildwheel upgrade required us to drop generating the CPython 3.13 free-threaded wheels. +Bug Fixes +--------- + +* The CFFI backend no longer crashes or fails with ``Dictionary mismatch`` + when multiple ``ZstdDecompressor`` instances use the same + ``ZstdCompressionDict``. It used to create a new decompression dictionary for + every decompressor and operation, freeing the previous one while other + decompressors could still be using it. (#361) + 0.25.0 (released 2025-09-14) ============================ diff --git a/tests/test_decompressor_decompress.py b/tests/test_decompressor_decompress.py index 602df6fe..07a82545 100644 --- a/tests/test_decompressor_decompress.py +++ b/tests/test_decompressor_decompress.py @@ -158,6 +158,35 @@ def test_dictionary_multiple(self): decompressed = dctx.decompress(compressed[i]) self.assertEqual(decompressed, sources[i]) + def test_dictionary_shared_by_decompressors(self): + samples = [] + for i in range(128): + samples.append(b"foo" * 64) + samples.append(b"bar" * 64) + samples.append(b"foobar" * 64) + samples.append(b"qwert" * 64) + samples.append(b"yuiop" * 64) + samples.append(b"asdfg" * 64) + samples.append(b"hijkl" * 64) + + d = zstd.train_dictionary(8192, samples) + + orig = b"foobar" * 16384 + cctx = zstd.ZstdCompressor(level=1, dict_data=d) + compressed = cctx.compress(orig) + + for _ in range(8): + # The reader's decompression context is bound to the dictionary + # when the reader is created. Creating another decompressor for + # the same dictionary must not invalidate it. + reader = zstd.ZstdDecompressor(dict_data=d).stream_reader( + compressed + ) + dctx = zstd.ZstdDecompressor(dict_data=d) + + self.assertEqual(reader.read(), orig) + self.assertEqual(dctx.decompress(compressed), orig) + def test_max_window_size(self): with open(__file__, "rb") as fh: source = fh.read() diff --git a/zstandard/backend_cffi.py b/zstandard/backend_cffi.py index 9d830cf1..10058139 100644 --- a/zstandard/backend_cffi.py +++ b/zstandard/backend_cffi.py @@ -85,6 +85,7 @@ import io import os +import threading from ._cffi import ( # type: ignore ffi, @@ -2593,6 +2594,10 @@ def get_frame_parameters(data, format=FORMAT_ZSTD1): return FrameParameters(params[0]) +# Serializes the lazy creation of ZstdCompressionDict._ddict. +_DDICT_LOCK = threading.Lock() + + class ZstdCompressionDict(object): """Represents a computed compression dictionary. @@ -2747,21 +2752,35 @@ def precompute_compress(self, level=0, compression_params=None): @property def _ddict(self): - ddict = lib.ZSTD_createDDict_advanced( - self._data, - len(self._data), - lib.ZSTD_dlm_byRef, - self._dict_type, - lib.ZSTD_defaultCMem, - ) + # Like the C backend, create the DDict once and keep it as long as + # the dictionary: decompression contexts only reference it, so + # replacing it would free a DDict that another decompressor may + # still be using. The property takes precedence over the instance + # __dict__, so the cached value must be looked up explicitly. + ddict = self.__dict__.get("_ddict") + if ddict is not None: + return ddict + + with _DDICT_LOCK: + ddict = self.__dict__.get("_ddict") + if ddict is not None: + return ddict + + ddict = lib.ZSTD_createDDict_advanced( + self._data, + len(self._data), + lib.ZSTD_dlm_byRef, + self._dict_type, + lib.ZSTD_defaultCMem, + ) - if ddict == ffi.NULL: - raise ZstdError("could not create decompression dict") + if ddict == ffi.NULL: + raise ZstdError("could not create decompression dict") - ddict = ffi.gc( - ddict, lib.ZSTD_freeDDict, size=lib.ZSTD_sizeof_DDict(ddict) - ) - self.__dict__["_ddict"] = ddict + ddict = ffi.gc( + ddict, lib.ZSTD_freeDDict, size=lib.ZSTD_sizeof_DDict(ddict) + ) + self.__dict__["_ddict"] = ddict return ddict