You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
gh-143620: Fix race condition in test_traceback_when_child_process_terminates_abruptly - #143621
There is a race condition in test_concurrent_futures.test_process_pool. The test asserts that a BrokenProcessPool exception has a __cause__ immediately after future.result() raises.
In concurrent.futures.process, the _terminate_broken method sets the exception on the future before it attaches the _RemoteTraceback to the __cause__ attr. Sometimes the test thread wakes up and inspects the exception while __cause__ is still None, which leads to a test failure. I was able to repro (see issue for more details) by injecting a delay in _terminate_broken between the set_exception() call and the __cause__ assignment. This causes a 100% failure rate with AssertionError: None is not an instance of _RemoteTraceback.
My proposed fix is to update the test to use support.sleeping_retry to wait for the __cause__ to be populated. This seems to be the pattern elsewhere in the test suite.
I think the race condition analysis here may be slightly off. Looking at _terminate_broken(), bpe.__cause__ is assigned on line 493 beforeset_exception() on line 498, so the test thread shouldn't be able to see __cause__ as None due to ordering.
The actual issue seems to be that when cause is None (the os._exit path), p.exitcode can still return None at line 487 because waitpid() hasn't collected the exit status yet — even though the process sentinel has already fired. This means errors stays empty, cause_str stays None, and bpe.__cause__ is never set at all.
A sleeping_retry in the test won't help here because nobody goes back to set __cause__ after the fact — once _terminate_broken() finishes, the exception is final.
A fix in _terminate_broken() itself might work better — calling p.join(timeout=3) before reading p.exitcode would ensure waitpid() has collected the exit status:
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes gh-143620
There is a race condition in
test_concurrent_futures.test_process_pool. The test asserts that aBrokenProcessPoolexception has a__cause__immediately afterfuture.result()raises.In
concurrent.futures.process, the_terminate_brokenmethod sets the exception on the future before it attaches the_RemoteTracebackto the__cause__attr. Sometimes the test thread wakes up and inspects the exception while__cause__is stillNone, which leads to a test failure. I was able to repro (see issue for more details) by injecting a delay in_terminate_brokenbetween theset_exception()call and the__cause__assignment. This causes a 100% failure rate withAssertionError: None is not an instance of _RemoteTraceback.My proposed fix is to update the test to use
support.sleeping_retryto wait for the__cause__to be populated. This seems to be the pattern elsewhere in the test suite.