-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-109746: Make _thread.start_new_thread
delete state of new thread on its startup failure
#109761
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
gh-109746: Make _thread.start_new_thread
delete state of new thread on its startup failure
#109761
Conversation
If Python fails to start newly created thread due to failure of underlying PyThread_start_new_thread() call, its state should be removed from interpreter' thread states list to avoid its double cleanup * make 'PyThreadState_Delete' accept unbound thread state * in 'thread_PyThread_start_new_thread', delete state of new thread from interpreter if 'PyThread_start_new_thread' call fails
_thread.start_new_thread
delete state of new thread on its startup failure_thread.start_new_thread
delete state of new thread on its startup failure
The test did not fail on non-patched code, failed on Windows, and hanged on macOS. I rewrote it. According to a comment in a similar test, |
!buildbot bsd |
🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 1f3ecd9 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
Applying a version of the fix in this PR solves an issue I had where all processes with large thread counts would abort at finalization (particularly annoying when you're trying to fuzz CPython). |
!buildbot bsd |
🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 185d5a6 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
!buildbot bsd |
🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 55a8237 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
Thanks @chgnrdv for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, @chgnrdv and @serhiy-storchaka, I could not cleanly backport this to
|
… new thread on its startup failure (pythonGH-109761) If Python fails to start newly created thread due to failure of underlying PyThread_start_new_thread() call, its state should be removed from interpreter' thread states list to avoid its double cleanup. (cherry picked from commit ca3ea9a) Co-authored-by: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
GH-127171 is a backport of this pull request to the 3.13 branch. |
… new thread on its startup failure (pythonGH-109761) If Python fails to start newly created thread due to failure of underlying PyThread_start_new_thread() call, its state should be removed from interpreter' thread states list to avoid its double cleanup. (cherry picked from commit ca3ea9a) Co-authored-by: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
GH-127173 is a backport of this pull request to the 3.12 branch. |
…hread on its startup failure (GH-109761) (GH-127171) If Python fails to start newly created thread due to failure of underlying PyThread_start_new_thread() call, its state should be removed from interpreter' thread states list to avoid its double cleanup. (cherry picked from commit ca3ea9a) Co-authored-by: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com>
…hread on its startup failure (GH-109761) (GH-127173) If Python fails to start newly created thread due to failure of underlying PyThread_start_new_thread() call, its state should be removed from interpreter' thread states list to avoid its double cleanup. (cherry picked f 8000 rom commit ca3ea9a) Co-authored-by: Radislav Chugunov <52372310+chgnrdv@users.noreply.github.com>
The test is unstable on macOS and FreeBSD: it fails randomly. Examples:
Example of failure on FreeBSD:
|
On Linux, RLIMIT_NPROC is the maximum number of threads. On FreeBSD, RLIMIT_NPROC is the maximum number of processes. |
I suggest to make the test specific to Linux, or use another approach than RLIMIT_NPROC. |
Ah, there is a race condition here. Should be fixed by #127299. |
…read on its startup failure (pythonGH-109761) If Python fails to start newly created thread due to failure of underlying PyThread_start_new_thread() call, its state should be removed from interpreter' thread states list to avoid its double cleanup. Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Fixes #109746.
If Python fails to start newly created thread due to failure of underlying PyThread_start_new_thread() call, its state should be removed from interpreter' thread states list to avoid its double cleanup.
PyThreadState_Delete
accept unbound thread statethread_PyThread_start_new_thread
, delete state of new thread from interpreter ifPyThread_start_new_thread
call fails