Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-87135: Raise PythonFinalizationError when joining a blocked daemon thread#130402
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
…daemon threadIf `Py_IsFinalizing()` is true, non-daemon threads (other than the current one)are done, and daemon threads are prevented from running, so theycannot finalize themselves and become done. Joining them without timeoutwould block forever.Raise PythonFinalizationError instead of hanging.Seepythongh-123940 for a use case: calling `join()` from `__del__`. This isill-advised, but an exception should at least make it easier to diagnose.
This seems like a good idea to me.
|
I agree with Sam. |
True. Threads that are already done can be joined normally.
In a finalizer, wouldn't it be OK to wait a bit for graceful termination (using Raising an exception would mean you skip that teardown, unless you have a IOW, to me, the reasoning is not as clear-cut here as in the “hang the only Python thread that can run” case.
Well... You can even write an infinite loop without any join at all! :) Iwrote the code to hang even with timeout; I'll update the PR if you still think that's the way to go. |
Updated to raise even with timeout. |
| if (ThreadHandle_ident(self) == PyThread_get_thread_ident_ex()) { | ||
| // PyThread_join_thread() would deadlock or error out. | ||
| PyErr_SetString(ThreadError, "Cannot join current thread"); | ||
| return -1; | ||
| } | ||
| if (Py_IsFinalizing()) { |
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.
Is this code path taken by all threads, or only daemon threads?
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.
It's taken by the thread that calledPy_FinalizeEx.
WhenPy_IsFinalizing is true, all other threads than the one that calledPy_FinalizeEx are daemonic and they cannot call Python API (includingThreadHandle_join).
So,self must be a daemon thread.
I'll merge on ~Friday if there are no objections. |
... And I went offline for a month after writing that. I'm back now; merging. |
4ebbfcf intopython:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
If
Py_IsFinalizing()is true, non-daemon threads (other than the current one) are done, and daemon threads are prevented from acquiring GIL (or thread state), so they cannot finalize themselves and become done. Joining them without timeout would block forever.Raise PythonFinalizationError instead of hanging.
Seegh-123940 for a real-world use case: calling
join()from__del__.Doing this is still ill-advised, but an exception should at least make the issue easier to diagnose.
📚 Documentation preview 📚:https://cpython-previews--130402.org.readthedocs.build/