-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…daemon thread If `Py_IsFinalizing()` is true, non-daemon threads (other than the current one) are done, and daemon threads are prevented from running, so they cannot finalize themselves and become done. Joining them without timeout would block forever. Raise PythonFinalizationError instead of hanging. See pythongh-123940 for a use case: calling `join()` from `__del__`. This is ill-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! :) I wrote 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 called Py_FinalizeEx
.
When Py_IsFinalizing
is true, all other threads than the one that called Py_FinalizeEx
are daemonic and they cannot call Python API (including ThreadHandle_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. |
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.
See gh-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/