New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove .stopped Event from Thread internals #63184
Comments
As discussed in bpo-18808, now that we're checking for a tstate lock, the Thread._stopped Event has become an "attractive nuisance". The attached patch removes it. This simplifies .join() and .is_alive(), and restores pre-18808 .join(timeout) endcase behavior (i.e., if join returns before the timeout expires, .is_alive() will always be False). Since this doesn't add any new locks, I hope it won't create more problems with fork tests - but running on Windows I wouldn't know ;-) |
Hmm.. I'm getting the following failures: ====================================================================== Traceback (most recent call last):
File "/home/antoine/cpython/default/Lib/test/test_threading.py", line 726, in test_4_joining_across_fork_in_worker_thread
self.assertScriptHasOutput(script, "end of main\n")
File "/home/antoine/cpython/default/Lib/test/test_threading.py", line 651, in assertScriptHasOutput
rc, out, err = assert_python_ok("-c", script)
File "/home/antoine/cpython/default/Lib/test/script_helper.py", line 55, in assert_python_ok
return _assert_python(True, *args, **env_vars)
File "/home/antoine/cpython/default/Lib/test/script_helper.py", line 46, in _assert_python
"stderr follows:\n%s" % (rc, err.decode('ascii', 'ignore')))
AssertionError: Process return code is 1, stderr follows:
Traceback (most recent call last):
File "<string>", line 29, in <module>
AttributeError: 'Thread' object has no attribute '_stopped' ====================================================================== Traceback (most recent call last):
File "/home/antoine/cpython/default/Lib/test/test_threading.py", line 784, in test_5_clear_waiter_locks_to_avoid_crash
self.assertScriptHasOutput(script, output)
File "/home/antoine/cpython/default/Lib/test/test_threading.py", line 651, in assertScriptHasOutput
rc, out, err = assert_python_ok("-c", script)
File "/home/antoine/cpython/default/Lib/test/script_helper.py", line 55, in assert_python_ok
return _assert_python(True, *args, **env_vars)
File "/home/antoine/cpython/default/Lib/test/script_helper.py", line 46, in _assert_python
"stderr follows:\n%s" % (rc, err.decode('ascii', 'ignore')))
AssertionError: Process return code is 1, stderr follows:
Traceback (most recent call last):
File "<string>", line 29, in <module>
AttributeError: 'Thread' object has no attribute '_stopped' |
Looks like it's mostly the tests trying to check an implementation detail that doesn't exist anymore :-) |
I agree with your diagnosis. Unfortunately, I can't test the fork stuff. Well, OK, I actually think that's fortunate (for me ;-) ). If you can see a quick way to fix these, please do. I'm going to vanish for about 2 hours now. |
I think the fix is simply to remove the outdated tests. I've been running test_threading in a loop with the (attached) updated patch, and there hasn't been a single failure. |
New changeset aff959a3ba13 by Tim Peters in branch 'default': |
Thanks, Antoine! I pushed this change, figuring that even if some buildbots are still unhappy about 18808, getting rid of the Event can't make them more unhappy, and may make them happier. |
Antoine, could I bother you to try the attached cleanup.patch? It looks harmless to me, but when I checked it in the Unix-y buildbots failed the thread+fork tests again :-( Two examples: http://buildbot.python.org/all/builders/x86%20Gentoo%203.x/builds/4914/steps/test/logs/stdio Worse, the buildbot slaves appeared never to finish running then - I had to stop them (via the "stop build" button). Maybe I'm just tired, but I've stared & stared at this and just don't see what's going wrong :-( |
Here is a fixed patch (works here, at least :-)). |
Well - I remain baffled, but am grateful for the patch - thanks :-) |
I suppose we can close this issue now? |
Yes - and I just closed 18808 :-) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: