gh-140746: Fix Thread.start() that can hang indefinitely - v2#144750
Open
YvesDup wants to merge 3 commits intopython:mainfrom
Open
gh-140746: Fix Thread.start() that can hang indefinitely - v2#144750YvesDup wants to merge 3 commits intopython:mainfrom
YvesDup wants to merge 3 commits intopython:mainfrom
Conversation
1 task
bkap123
reviewed
Feb 18, 2026
Contributor
bkap123
left a comment
There was a problem hiding this comment.
Left a couple of small comments on semantics.
|
|
||
| // Handles state transitions according to the following diagram: | ||
| // | ||
| // NOT_STARTED -> STARTING -> RUNNING -> DONE |
Contributor
There was a problem hiding this comment.
This comment should be updated now that there can be a FAILED state. Possibly just replace error with FAILED.
| // NOT_STARTED -> STARTING -> RUNNING -> DONE | ||
| // | ^ | ||
| // | | | ||
| // +----- error --------+ |
Contributor
There was a problem hiding this comment.
Suggested change
| // +----- FAILED --------+ |
|
|
||
| PyMutex mutex; | ||
|
|
||
| PyEvent thread_is_bootstraped; |
Contributor
There was a problem hiding this comment.
"bootstraped" is misspelled everywhere (should be bootstrapped).
| PyErr_FormatUnraisable( | ||
| "Exception ignored in thread started by %R", boot->func); | ||
| } | ||
| // Notify that the bootstraped is done and failed (e.g. Memory error). |
Contributor
There was a problem hiding this comment.
I think this makes more sense:
Suggested change
| // Notify that the bootstraped is done and failed (e.g. Memory error). | |
| // Notify that bootstrap is done and failed (e.g. Memory error). |
| @@ -0,0 +1,2 @@ | |||
| Fix :func:`threading.Thread.start` that can hang indefinitely in case of heap memory exhaustion | |||
| during initialization of the new thread. | |||
Contributor
There was a problem hiding this comment.
Maybe add something about the new FAILED state that is managed by the C code?
| return NULL; | ||
| } | ||
|
|
||
| // gh-140746: catch error before thread really start |
Contributor
There was a problem hiding this comment.
Suggested change
| // gh-140746: catch error before thread really start | |
| // gh-140746: catch error before thread really starts |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is based on the @ryv-odoo PR. I would have preferred to start from @ryv-odoo's PR branch rather than duplicating his code, but I don't know if that's possible.
Following the @colesbury comment,
I submit this PR that includes the waiting in C the module and raise a runtime exception when the thread handle state is set to failed. This exception is treated in the
Thread.start.