bpo-39995: Fix race condition in ProcessPoolExecutor._ThreadWakeup by aeros · Pull Request #19751 · python/cpython
Conversation
Addresses a race condition in ProcessPoolExecutor._ThreadWakeup by using a threading.Event to ensure either end of its pipe is not closed while it is actively sending or receiving bytes.
Fixes an intermittent failure in test_concurrent_futures.ProcessPoolSpawnProcessPoolExecutorTest.test_killed_child, and aims to fix some of the others reported in the bpo issue.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would favour a non-locking solution (hence my suggestion on the bpo issue), but this PR is faulty anyway. You could very well have the following execution sequence:
Thread A Thread B
-------- ---------
<Enters wakeup()>
if not self._closed: # Success
<Enters close()>
self._closed = True
self._not_running.wait() # Success
self._writer.close()
self._not_running.clear()
self._writer.send_bytes(b"") # Oops
When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.
And if you don't make the requested changes, you will be poked with soft cushions!
@pitrou Ah, I had somehow not considered that scenario and got a bit carried away with the surface-level success of it appearing to resolve test_killed_child failure locally. In retrospect, I can see now that this PR would simply make the race condition slightly less likely to occur, but does not properly fix it. Thanks for the feedback; I'll close the PR.
this PR is faulty anyway
I wrote PR #19758 which is similar to this one, but doesn't have this race condition.
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