Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-104341: Fix threading Module Shutdown#104560
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
base:main
Are you sure you want to change the base?
gh-104341: Fix threading Module Shutdown#104560
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Modules/_threadmodule.c Outdated
| PyThread_acquire_lock(threads->mutex,WAIT_LOCK); | ||
| while (threads->head!=NULL) { | ||
| PyThread_release_lock(threads->mutex); | ||
| // XXX Sleep? | ||
| PyThread_acquire_lock(threads->mutex,WAIT_LOCK); | ||
| } | ||
| PyThread_release_lock(threads->mutex); |
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.
FYI, this doesn't feel right. I'm going to look at what my options are. A condvar would work, but that doesn't seem to be something we do in CPython outside the GIL.
(I'm also open to ideas.)
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've come up with a slightly better approach.
bedevere-bot commentedMay 16, 2023
🤖 New build scheduled with the buildbot fleet by@ericsnowcurrently for commit51d960e 🤖 If you want to schedule another build, you need to add the🔨 test-with-buildbots label again. |
Modules/_threadmodule.c Outdated
| staticstructPyModuleDefthread_module; | ||
| /* threads owned by the modulo */ |
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.
typo: module?
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.
fixed
ericsnowcurrently commentedMay 17, 2023
FTR, I ran the buildbots on commit51d960e:https://buildbot.python.org/all/#/changes/22637. |
ericsnowcurrently commentedMay 17, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Based on CI and the buildbots, I'm thinking there may be something wrong with this PR on macOS. I'll take a look. Run failed when multiple fork-related tests timed out:
On the "x86-64 macOS PR" builder, the run was cancelled 3 times: |
ericsnowcurrently commentedMay 17, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
CI run 1(https://github.com/python/cpython/actions/runs/4996253064/jobs/8949214929) 12 test modules failed before the CI job was canceled. (details)test module timed out after 20 minutes (1200s):
individual tests timed out:
still running:
CI run 2(https://github.com/python/cpython/actions/runs/4997616864/jobs/8952165246) 12 test modules failed before the CI job was canceled. (details)test module timed out after 20 minutes (1200s):
individual tests timed out:
still running:
Buildbot (ARM64 macOS PR)(https://buildbot.python.org/all/#/builders/721/builds/1163/steps/5/logs/stdio) 25 test modules failed (details)test module timed out after 15 minutes (900s):
individual tests timed out:
|
arhadthedev commentedMay 17, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I reran the job and got the same result (the tests are not done yet but started to time out already). |
Uh oh!
There was an error while loading.Please reload this page.
This should fix the frequent crashes in test_threading and test__xxsubinterpreters.
There is some duplication with the threading module which could be removed after this lands. We can also eliminate
PyThreadState.on_deleteandPyInterpreterState.threads.count, both of which are cases where the state of the threading module leaked out into the runtime.(Also seegh-63008 andgh-18296.)