bpo-45274: Fix Thread._wait_for_tstate_lock() race condition #28532
Conversation
Fix a race condition in the Thread.join() method of the threading module. If the function is interrupted by a signal and the signal handler raises an exception, make sure that the thread remains in a consistent state to prevent a deadlock.
| # was interrupted with an exception before reaching the | ||
| # lock.release(). It can happen if a signal handler raises an | ||
| # exception, like CTRL+C which raises KeyboardInterrupt. | ||
| lock.release() |
There was a problem hiding this comment.
I don't think that this code is 100% reliable if lock.release() gets interrupted by a second exception (ex: raised by a second signal: fatality!). Maybe a context manager could be used. But I'm exhausted, I will think about this code after a good night :-)
There was a problem hiding this comment.
I believe we've still got bugs in the context manager exit implementation where even it can miss something in this scenario and not call __exit__. https://bugs.python.org/issue29988
This came up during the dev sprint in 2017 when working on the interpreter loop.
So this PR, while not technically a fix, is at least an improvement and should help the single Ctrl-C KeyboardInterrupt case.
There was a problem hiding this comment.
The whole function should be reimplemented in C to better control how signals are handled.
In pure Python, I don't think that it's possible to fully control handle any possible exception at any line number.
@serhiy-storchaka proposed to rewrite acquire()+release() in C to make sure that at least the lock remains consistent: https://bugs.python.org/issue45274#msg402532 So it doesn't handle exceptions in the _stop() method.
There was a problem hiding this comment.
But I only wanted to enhance the code, I'm not interested to rewrite threading.Thread in C.
|
cc @pablogsal @ambv @corona10 @serhiy-storchaka: Here is an interesting race condition in the threading module ;-) Making sure that we restore the Thread object in a consistent state is challenging. My fix should make these code less bad (more reliable, but not 100% reliable). |
|
cc @gpshead |
vstinner
added
needs backport to 3.9
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10. |
|
GH-28579 is a backport of this pull request to the 3.9 branch. |
|
GH-28580 is a backport of this pull request to the 3.10 branch. |
…H-28532) Fix a race condition in the Thread.join() method of the threading module. If the function is interrupted by a signal and the signal handler raises an exception, make sure that the thread remains in a consistent state to prevent a deadlock. (cherry picked from commit a22be49) Co-authored-by: Victor Stinner <[email protected]>
…H-28532) Fix a race condition in the Thread.join() method of the threading module. If the function is interrupted by a signal and the signal handler raises an exception, make sure that the thread remains in a consistent state to prevent a deadlock. (cherry picked from commit a22be49) Co-authored-by: Victor Stinner <[email protected]>
Fix a race condition in the Thread.join() method of the threading module. If the function is interrupted by a signal and the signal handler raises an exception, make sure that the thread remains in a consistent state to prevent a deadlock. (cherry picked from commit a22be49) Co-authored-by: Victor Stinner <[email protected]>
… (GH-28580) Fix a race condition in the Thread.join() method of the threading module. If the function is interrupted by a signal and the signal handler raises an exception, make sure that the thread remains in a consistent state to prevent a deadlock. (cherry picked from commit a22be49) Co-authored-by: Victor Stinner <[email protected]>
Fix a race condition in the Thread.join() method of the threading
module. If the function is interrupted by a signal and the signal
handler raises an exception, make sure that the thread remains in a
consistent state to prevent a deadlock.
https://bugs.python.org/issue45274