-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
test_zstd
failed on ubuntu with free-threading
#133885
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
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?
Details
From below, it seems that
|
I believe @Rogdham's analysis is correct. I think unfortunately we will have to require that |
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. |
Should it be only disabled for free-threading? |
I believe so, it passed on all other CIs. The merged pr unnecessarily skips it without that consideration. |
@StanFromIreland it is not "unnecessarily skips", it skips tests that were failing :) Previously these tests were only run on Lines 2433 to 2439 in 8cf4947
Lines 2472 to 2476 in 8cf4947
Now - they are not executed at all, untill we fix them / code :) |
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. |
Crash report
Link: https://github.com/python/cpython/actions/runs/14954410814/job/42008158034?pr=133876
Linked PRs
test_compress_locking
intest_zstd
#133943test_compress_locking
intest_zstd
(GH-133943) #133949The text was updated successfully, but these errors were encountered: