bpo-39995: Fix race condition in ProcessPoolExecutor._ThreadWakeup by aeros · Pull Request #19751 · python/cpython

Conversation

@aeros

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.

https://bugs.python.org/issue39995

pitrou

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

@bedevere-bot

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!

@aeros

@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.

@vstinner

this PR is faulty anyway

I wrote PR #19758 which is similar to this one, but doesn't have this race condition.

Labels