gh-133885: Disallow sharing zstd (de)compressor contexts #134253
Conversation
According to the zstd author, it is not possible to share Zstandard objects across thread boundaries. To resolve this, we check if the object was created on the current thread and raise a RuntimeError if it is not. The tests are updated to ensure that the error is raised if a (de)compression context is shared across threads.
| @@ -52,4 +52,21 @@ extern void | |||
| set_parameter_error(const _zstd_state* const state, int is_compress, | |||
| int key_v, int value_v); | |||
|
|
|||
| static inline int | |||
| check_object_shared(PyObject *ob, char *type) | |||
There was a problem hiding this comment.
I think this is probably not be the right way to implement this check? Access from different threads could happen regardless of free-threading builds.
I think the concept the extension module might want to enforce is something like getting the current thread id (we have a python c api for this IIRC) upon first "use" of the object, saving that in the object state, and checking that current thread id == that thread id for subsequent uses.
first use could be construction... but unless the zstandard C API requires that, it may be better to consider first use the first time that ZStdCompressor is actually used by a C API. This allows for the pattern of:
A compressor or decompressor is created by one thread - and added to a thread pool or work queue to be actually used and exhausted - exclusively - in another thread.
|
After talking with Thomas, I think I'm going to try a different approach. |
It is not possible to share Zstandard objects across thread boundaries. To resolve this, we check if the object was created on the current thread and raise a
RuntimeErrorif it is not. Wasn't totally sure what exception to raise to communicate the issue, so ifRuntimeErrorisn't correct, please suggestion another option.The tests are updated to ensure that the error is raised if a (de)compression context is shared across threads.
(as requested, cc @ngoldbaum )
test_zstdfailed on ubuntu with free-threading #133885