Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-133885: Disallow sharing zstd (de)compressor contexts#134253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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. |
Uh oh!
There was an error while loading.Please reload this page.
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