Conversation
…hread The test script ended at ``w.start()``, so the ``os.fork()`` in ``worker()`` raced the interpreter shutdown that begins as soon as the main thread falls off the end of the script. When the fork loses that race it happens inside ``threading._shutdown()``, which marks the main thread as stopped before joining non-daemon threads. The child then inherits a main thread that was already marked as stopped in the parent, so ``mainthread.join()`` in ``joiningfunc()`` returns for the wrong reason and the test stops checking what its own comment describes: "In the forked process, the main Thread object must be marked as stopped." Join ``w`` so that the main thread stays alive until the fork has happened, and check in ``worker()`` that the main thread really is still alive before forking. Without that check the test passes either way, so losing the join again would go unnoticed. With ``time.sleep(0.1)`` added at the start of ``worker()`` (the reproducer from the issue), the fork previously saw ``threading._SHUTTING_DOWN`` true and the main thread already marked done; it now sees both false, and the main thread handle is marked done only in the child.
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #116612.
test_3_join_in_forked_from_threadforks from a worker thread while the main thread is running off the end of the script, so whetherfork()lands before or after interpreter shutdown begins is a race. When it lands after, the main thread is already marked stopped in the parent, the child inherits that, andmainthread.join()in the child returns for the wrong reason — the test passes without testing anything.Change
Two lines of substance in
Lib/test/test_threading.py:w.join()at the end of the script, so the main thread stays alive until the worker has forked. That is what actually establishes the ordering.fork(), exiting3and printing to stderr if it is not. That is what makes a future regression loud instead of silently green.What I can and cannot demonstrate
Being straight about this, because it changes how much the diff is worth:
The race does not reproduce unaided on this machine. With
w.join()removed, the test still passed 12 runs out of 12 — the fork happens to win every time here. So there is no red-to-green transition to show, and that is exactly why the guard is the pin rather than a failing assertion.The hazard is real, though, and the guard catches it. Delaying the fork by 0.5 s inside the worker, which is what a loaded machine or a slower interpreter does for free:
w.join()present, no delayw.join()removed, no delayw.join()removed, 0.5 s beforefork()main thread stopped before fork()w.join()present, 0.5 s beforefork()So the main thread genuinely can be gone by the time the worker forks,
w.join()is what prevents it, and without the guard that state would have gone unnoticed.News
No entry: the change is confined to
Lib/test/test_threading.pyand nothing user-visible moves. Of the last 40 commits touchingLib/test/, the 8 that touched onlyLib/test/all landed without aMisc/NEWS.dentry.bedevere/newswill therefore want theskip newslabel, which I cannot apply — happy to add an entry underMisc/NEWS.d/next/Tests/instead if you would rather have one.