Skip to content

Commit 14918f7

Browse files
[3.14] gh-160535: Keep the ZstdDict alive until the zstd context is freed (GH-160536) (#160636)
gh-160535: Keep the ZstdDict alive until the zstd context is freed (GH-160536) The garbage collector could release a ZstdCompressor's dictionary, or the digested dictionaries cached by the ZstdDict, while worker threads of the compression context were still reading them. They are now only released on deallocation, after the context has been freed. ANT-2026-CT3P1CSJ + Visit the type in the compressor and decompressor traverse; move NEWS to Library (cherry picked from commit b949033) Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent 6a48876 commit 14918f7

5 files changed

Lines changed: 54 additions & 28 deletions

File tree

‎Lib/test/test_zstd.py‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,12 @@
77
import os
88
import unittest
99
import tempfile
10+
import textwrap
1011
import threading
1112

13+
from test import support
1214
from test.support.import_helper import import_module
15+
from test.support.script_helper import assert_python_ok
1316
from test.support import threading_helper
1417
from test.support import _1M
1518
from test.support import Py_GIL_DISABLED
@@ -1549,6 +1552,42 @@ def test_len(self):
15491552
self.assertEqual(len(TRAINED_DICT), len(TRAINED_DICT.dict_content))
15501553
self.assertIn(str(len(TRAINED_DICT)), str(TRAINED_DICT))
15511554

1555+
@unittest.skipUnless(SUPPORT_MULTITHREADING,
1556+
"zstd build doesn't support multi-threaded compression")
1557+
@support.requires_subprocess()
1558+
def test_collect_cycle_with_busy_compressor(self):
1559+
# The dictionary must outlive the compression context when both are
1560+
# collected by the garbage collector while a worker thread is still
1561+
# using the dictionary. Whether the failure is detected depends on
1562+
# the memory allocator, so use a fresh process for each case.
1563+
code = textwrap.dedent("""
1564+
import gc, random, sys
1565+
from compression.zstd import (CompressionParameter,
1566+
ZstdCompressor, ZstdDict)
1567+
1568+
class Cycle:
1569+
pass
1570+
1571+
job_size = 1 << 20
1572+
content = random.randbytes(1 << 10) * (1 << 8)
1573+
zd = ZstdDict(content, is_raw=True)
1574+
options = {CompressionParameter.compression_level: 10,
1575+
CompressionParameter.nb_workers: 1,
1576+
CompressionParameter.job_size: job_size}
1577+
c = ZstdCompressor(options=options,
1578+
zstd_dict=getattr(zd, sys.argv[1]))
1579+
cycle = Cycle()
1580+
cycle.cycle = cycle
1581+
cycle.compressor = c
1582+
# Returns while the worker thread is compressing the first job.
1583+
c.compress(random.randbytes(1 << 10) * ((job_size >> 10) + 64))
1584+
del zd, c, cycle
1585+
gc.collect()
1586+
""")
1587+
for attr in 'as_digested_dict', 'as_prefix':
1588+
with self.subTest(attr=attr):
1589+
assert_python_ok('-c', code, attr)
1590+
15521591
class FileTestCase(unittest.TestCase):
15531592
def setUp(self):
15541593
self.DECOMPRESSED_42 = b'a'*42
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a crash in :mod:`compression.zstd` when the garbage collector frees a
2+
:class:`~compression.zstd.ZstdCompressor` that uses worker threads together
3+
with its :class:`~compression.zstd.ZstdDict` before the compression of
4+
already submitted data has finished.

‎Modules/_zstd/compressor.c‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -765,17 +765,13 @@ static int
765765
ZstdCompressor_traverse(PyObject *ob, visitproc visit, void *arg)
766766
{
767767
ZstdCompressor *self = ZstdCompressor_CAST(ob);
768+
Py_VISIT(Py_TYPE(ob));
768769
Py_VISIT(self->dict);
769770
return 0;
770771
}
771772

772-
static int
773-
ZstdCompressor_clear(PyObject *ob)
774-
{
775-
ZstdCompressor *self = ZstdCompressor_CAST(ob);
776-
Py_CLEAR(self->dict);
777-
return 0;
778-
}
773+
/* No tp_clear: the compression context (and its worker threads) uses the
774+
dictionary until ZstdCompressor_dealloc() has freed the context. */
779775

780776
static PyType_Slot zstdcompressor_slots[] = {
781777
{Py_tp_new, _zstd_ZstdCompressor_new},
@@ -784,7 +780,6 @@ static PyType_Slot zstdcompressor_slots[] = {
784780
{Py_tp_members, ZstdCompressor_members},
785781
{Py_tp_doc, (void *)_zstd_ZstdCompressor_new__doc__},
786782
{Py_tp_traverse, ZstdCompressor_traverse},
787-
{Py_tp_clear, ZstdCompressor_clear},
788783
{0, 0}
789784
};
790785

‎Modules/_zstd/decompressor.c‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -688,18 +688,13 @@ static int
688688
ZstdDecompressor_traverse(PyObject *ob, visitproc visit, void *arg)
689689
{
690690
ZstdDecompressor *self = ZstdDecompressor_CAST(ob);
691+
Py_VISIT(Py_TYPE(ob));
691692
Py_VISIT(self->dict);
692693
return 0;
693694
}
694695

695-
static int
696-
ZstdDecompressor_clear(PyObject *ob)
697-
{
698-
ZstdDecompressor *self = ZstdDecompressor_CAST(ob);
699-
Py_CLEAR(self->dict);
700-
Py_CLEAR(self->unused_data);
701-
return 0;
702-
}
696+
/* No tp_clear: the decompression context references the dictionary until
697+
ZstdDecompressor_dealloc() has freed the context. */
703698

704699
static PyType_Slot ZstdDecompressor_slots[] = {
705700
{Py_tp_new, _zstd_ZstdDecompressor_new},
@@ -709,7 +704,6 @@ static PyType_Slot ZstdDecompressor_slots[] = {
709704
{Py_tp_getset, ZstdDecompressor_getset},
710705
{Py_tp_doc, (void *)_zstd_ZstdDecompressor_new__doc__},
711706
{Py_tp_traverse, ZstdDecompressor_traverse},
712-
{Py_tp_clear, ZstdDecompressor_clear},
713707
{0, 0}
714708
};
715709

‎Modules/_zstd/zstddict.c‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -240,16 +240,11 @@ ZstdDict_length(PyObject *ob)
240240
static int
241241
ZstdDict_traverse(PyObject *ob, visitproc visit, void *arg)
242242
{
243-
ZstdDict *self = ZstdDict_CAST(ob);
244-
Py_VISIT(self->c_dicts);
245-
return 0;
246-
}
247-
248-
static int
249-
ZstdDict_clear(PyObject *ob)
250-
{
251-
ZstdDict *self = ZstdDict_CAST(ob);
252-
Py_CLEAR(self->c_dicts);
243+
/* c_dicts is not visited and there is no tp_clear: it only maps ints
244+
to capsules, and the ZSTD_CDict instances they own must stay alive
245+
until ZstdDict_dealloc() as a compression context being collected
246+
together with this object may still use them. */
247+
Py_VISIT(Py_TYPE(ob));
253248
return 0;
254249
}
255250

@@ -262,7 +257,6 @@ static PyType_Slot zstddict_slots[] = {
262257
{Py_tp_doc, (void *)_zstd_ZstdDict_new__doc__},
263258
{Py_sq_length, ZstdDict_length},
264259
{Py_tp_traverse, ZstdDict_traverse},
265-
{Py_tp_clear, ZstdDict_clear},
266260
{0, 0}
267261
};
268262

0 commit comments

Comments
 (0)