gh-133885: Disallow sharing zstd (de)compressor contexts - #134253
gh-133885: Disallow sharing zstd (de)compressor contexts#134253emmatyping wants to merge 2 commits into
Conversation
Sorry, something went wrong.
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.
| 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.
Sorry, something went wrong.
|
After talking with Thomas, I think I'm going to try a different approach. |
Sorry, something went wrong.
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. Wasn't totally sure what exception to raise to communicate the issue, so if RuntimeError isn'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 )