Skip to content

Commit 27703ce

Browse files
[3.14] gh-160549: Fix crash when freeing a busy multi-threaded ZstdCompressor (GH-160550) (#160638)
gh-160549: Fix crash when freeing a busy multi-threaded ZstdCompressor (GH-160550) ZSTD_freeCCtx() frees a dictionary loaded with ZSTD_CCtx_loadDictionary() before it stops the worker threads. Make libzstd wait for unfinished jobs before freeing the compression context of a ZstdCompressor that uses worker threads and a ZstdDict. Works around facebook/zstd#4862 (cherry picked from commit 23a5ba1) Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent ce48e3f commit 27703ce

3 files changed

Lines changed: 77 additions & 0 deletions

File tree

‎Lib/test/test_zstd.py‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,58 @@ def test_zstd_multithread_compress(self):
350350
with ZstdFile(io.BytesIO(), 'w', options=options) as f:
351351
f.write(b)
352352

353+
@unittest.skipIf(not SUPPORT_MULTITHREADING,
354+
"zstd build doesn't support multi-threaded compression")
355+
def test_free_busy_multithread_compressor_with_dict(self):
356+
# A compressor can be freed while a worker thread is still
357+
# compressing with a dictionary that the compression context owns.
358+
# Whether a failure is detected depends on the memory allocator,
359+
# so use a fresh process for each case.
360+
code = textwrap.dedent("""
361+
import gc, random, sys
362+
from compression.zstd import (CompressionParameter,
363+
ZstdCompressor, ZstdDict)
364+
365+
job_size = 1 << 20
366+
zd = ZstdDict(random.randbytes(1 << 10) * (1 << 10), is_raw=True)
367+
options = {CompressionParameter.compression_level: 10,
368+
CompressionParameter.nb_workers: 1,
369+
CompressionParameter.job_size: job_size}
370+
c = ZstdCompressor(options=options, zstd_dict=zd)
371+
# Returns while the worker thread is compressing the first job.
372+
c.compress(random.randbytes(1 << 10) * ((job_size >> 10) + 64))
373+
if sys.argv[1] == 'error':
374+
# A failed call does not wait for the worker thread either.
375+
import _testcapi
376+
_testcapi.set_nomemory(0)
377+
try:
378+
c.compress(b'x')
379+
except MemoryError:
380+
pass
381+
finally:
382+
_testcapi.remove_mem_hooks()
383+
if sys.argv[1] == 'gc':
384+
# The garbage collector clears the compressor before
385+
# deallocating it.
386+
cycle = [c]
387+
cycle.append(cycle)
388+
del c, cycle
389+
gc.collect()
390+
elif sys.argv[1] != 'exit':
391+
del c
392+
# Otherwise the compressor is freed at interpreter exit.
393+
""")
394+
cases = ['del', 'gc', 'exit']
395+
try:
396+
import _testcapi # noqa: F401
397+
except ImportError:
398+
pass
399+
else:
400+
cases.append('error')
401+
for case in cases:
402+
with self.subTest(case=case):
403+
assert_python_ok('-c', code, case)
404+
353405
def test_compress_flushblock(self):
354406
point = len(THIS_FILE_BYTES) // 2
355407

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a crash when a :class:`compression.zstd.ZstdCompressor` that uses worker
2+
threads and a :class:`~compression.zstd.ZstdDict` is deallocated, or the
3+
interpreter exits, while data passed to
4+
:meth:`~compression.zstd.ZstdCompressor.compress` is still being compressed.

‎Modules/_zstd/compressor.c‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ typedef struct {
3636
/* (nbWorker >= 1) ? 1 : 0 */
3737
int use_multithread;
3838

39+
/* Nonzero if worker threads may still be compressing submitted data:
40+
set by any call that did not complete a frame, including a failed one */
41+
int mt_maybe_busy;
42+
3943
/* Compression level */
4044
int compression_level;
4145

@@ -347,6 +351,7 @@ _zstd_ZstdCompressor_new_impl(PyTypeObject *type, PyObject *level,
347351
}
348352

349353
self->use_multithread = 0;
354+
self->mt_maybe_busy = 0;
350355
self->compression_level = ZSTD_CLEVEL_DEFAULT;
351356
self->dict = NULL;
352357
self->lock = (PyMutex){0};
@@ -427,6 +432,18 @@ ZstdCompressor_dealloc(PyObject *ob)
427432

428433
/* Free compression context */
429434
if (self->cctx) {
435+
if (self->mt_maybe_busy) {
436+
/* ZSTD_freeCCtx() (as of libzstd 1.5.7) frees the context's own
437+
copy of a dictionary loaded with ZSTD_CCtx_loadDictionary()
438+
before it stops the worker threads, which may still be
439+
reading it. Starting a new frame makes libzstd wait for the
440+
jobs of the previous one, and with no input it starts none. */
441+
ZSTD_inBuffer in = {NULL, 0, 0};
442+
ZSTD_outBuffer out = {NULL, 0, 0};
443+
(void)ZSTD_CCtx_reset(self->cctx, ZSTD_reset_session_only);
444+
(void)ZSTD_compressStream2(self->cctx, &out, &in,
445+
ZSTD_e_continue);
446+
}
430447
ZSTD_freeCCtx(self->cctx);
431448
}
432449

@@ -637,6 +654,8 @@ _zstd_ZstdCompressor_compress_impl(ZstdCompressor *self, Py_buffer *data,
637654
/* Resetting cctx's session never fail */
638655
ZSTD_CCtx_reset(self->cctx, ZSTD_reset_session_only);
639656
}
657+
self->mt_maybe_busy = (self->use_multithread
658+
&& (ret == NULL || mode != ZSTD_e_end));
640659
PyMutex_Unlock(&self->lock);
641660

642661
return ret;
@@ -685,6 +704,8 @@ _zstd_ZstdCompressor_flush_impl(ZstdCompressor *self, int mode)
685704
/* Resetting cctx's session never fail */
686705
ZSTD_CCtx_reset(self->cctx, ZSTD_reset_session_only);
687706
}
707+
self->mt_maybe_busy = (self->use_multithread
708+
&& (ret == NULL || mode != ZSTD_e_end));
688709
PyMutex_Unlock(&self->lock);
689710

690711
return ret;

0 commit comments

Comments
 (0)