Repository navigation
test_zstd failed on ubuntu with free-threading #133885
Description
Activity
- addedtestsTests in the Lib/test dirTests in the Lib/test dirtype-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump
on May 11, 2025 This is a duplicate I believe, IIRC it is part of the meta issue.
cc @emmatyping
Good idea to create a dedicated issue, it will be clearer to do the analysis here without polluting the other one.
Below is my last comment on the topic pasted for visibility.
Could it be the following?
- By sharing the
ZstdCompressorinstance between the threads, we share the compression context of typeZSTD_CCtx - From a comment in
zstd.h:- “since v1.3.0,
ZSTD_CStreamandZSTD_CCtxare the same thing” - “For parallel execution, use one separate
ZSTD_CStreamper thread.”
- “since v1.3.0,
Details
From below, it seems that
ZSTD_compressStream_genericline 6165 is writing data at the location ofzcs->inBuff(zcsbeing the compression context), so if another threads does the same or set it to null, we have the issue.ThreadSanitizer:DEADLYSIGNAL #0 <null> <null> (libc.so.6+0x16c5c7) (BuildId: d056ce83eebe65ce7e52ecfa5af5363e4863d283) #1 memcpy <null> (python+0xe8c98) (BuildId: dea998379ba126e7933ccc59c8ddb0390562739e) #2 ZSTD_limitCopy /usr/src/debug/zstd/zstd-1.5.7/lib/compress/../common/zstd_internal.h:252:9 (libzstd.so.1+0x26eae) (BuildId: 9723b93a8052010d25908aaa6174df6de760859a) #3 ZSTD_compressStream_generic /usr/src/debug/zstd/zstd-1.5.7/lib/compress/zstd_compress.c:6165:39 (libzstd.so.1+0x26eae) #4 ZSTD_compressStream2 /usr/src/debug/zstd/zstd-1.5.7/lib/compress/zstd_compress.c:6540:5 (libzstd.so.1+0x26eae) #5 compress_impl /redacted/./Modules/_zstd/compressor.c:453:20 (_zstd.cpython-315t-x86_64-linux-gnu.so+0x77e0) (BuildId: 74b337c2def9ea3fa795cafd5a7f41ce493b69f9) #6 _zstd_ZstdCompressor_compress_impl /redacted/./Modules/_zstd/compressor.c:591:15 (_zstd.cpython-315t-x86_64-linux-gnu.so+0x8cbf) (BuildId: 74b337c2def9ea3fa795cafd5a7f41ce493b69f9) #7 _zstd_ZstdCompressor_compress /redacted/./Modules/_zstd/clinic/compressor.c.h:171:20 (_zstd.cpython-315t-x86_64-linux-gnu.so+0x8cbf) <redacted>Reacted by Emma Smith- By sharing the
- added3.14bugs and security fixesbugs and security fixes3.15bugs and security fixesbugs and security fixesextension-modulesC modules in the Modules dirC modules in the Modules dir
on May 11, 2025 I believe @Rogdham's analysis is correct. I think unfortunately we will have to require that
Zstd(De)compressorinstances be only used in one thread at a time and raise an exception if someone tries to share them across threads. That's currently whatpython-zstandarddocuments as allowed, and raising an exception is what indygreg/python-zstandard#243 is planning on doing.cc @ngoldbaum as you had thoughts on free threading and zstd.
I'm not sure I understand all the context, but if the test is crashing because it's not thread-safe and the fix isn't immediately obvious, I think the test should be disabled for now. It's no fun to have the CI fail on PRs because of unrelated tests.
Reacted by Emma SmithShould it be only disabled for free-threading?
3 remaining items
So the (de)compressors need to be kept to the same thread, so perhaps we can have them hold a lock and if another thread tries to acquire it then it raises an exception? I think something like this could be implemented in a lightweight way using the object's internal ob_mutex most likely.
Hi @emmatyping 👋
I wanted to add a bit more context on the PR you linked.
We are not planning to set an exception when a de/compressor is shared between threads in python-zstandard because the maintainer asked us to revert that, so you won't find it in the PR.
But there's a commit that partly implemented that behavior, in case you're interested in reading it: indygreg/python-zstandard@3e0ec59#diff-3db8262822d3b43cb97f0d1fa8151dd4f4f8b121fb35d473133a8552f2f7ae72Reacted by Emma SmithThis test crashes in the (default) GIL enabled build too (even more often than in the FT build!), but it was always skipped by:
Line 2436 in b430e92
@unittest.skipUnless(Py_GIL_DISABLED, 'this test can only possibly fail with GIL disabled') It's a good idea to include threading tests like you're doing here, but we should be testing both the FT and default builds, especially when interacting with C libraries.
Reacted by Emma Smith- added a commit that references this issue
on May 23, 2025 Going to go ahead and close this, as we haven't seen any test failures since the PR adding locks around (de)compression contexts and ZstdDict landed. Thanks all for the help in resolving this!
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Crash report
Link: https://github.com/python/cpython/actions/runs/14954410814/job/42008158034?pr=133876
Linked PRs
test_compress_lockingintest_zstd#133943test_compress_lockingintest_zstd(GH-133943) #133949last_modein gh-153852 #154407