Remove .stopped Event from Thread internals

[tim-one]

BPO	18984
Nosy	@tim-one, @pitrou
Dependencies	bpo-18808: Thread.join returns before PyThreadState is destroyed
Files	remove_stopped: remove .stopped from Thread remove_stopped_2.patch cleanup.patch cleanup2.patch

^{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:

assignee = 'https://github.com/tim-one'
closed_at = <Date 2013-09-09.20:09:39.577>
created_at = <Date 2013-09-08.21:46:26.426>
labels = ['library', 'performance']
title = 'Remove .stopped Event from Thread internals'
updated_at = <Date 2013-09-09.20:09:39.558>
user = 'https://github.com/tim-one'

bugs.python.org fields:

activity = <Date 2013-09-09.20:09:39.558>
actor = 'tim.peters'
assignee = 'tim.peters'
closed = True
closed_date = <Date 2013-09-09.20:09:39.577>
closer = 'tim.peters'
components = ['Library (Lib)']
creation = <Date 2013-09-08.21:46:26.426>
creator = 'tim.peters'
dependencies = ['18808']
files = ['31690', '31691', '31700', '31701']
hgrepos = []
issue_num = 18984
keywords = ['patch', 'needs review']
message_count = 12.0
messages = ['197325', '197326', '197327', '197328', '197330', '197337', '197341', '197378', '197382', '197386', '197394', '197395']
nosy_count = 3.0
nosy_names = ['tim.peters', 'pitrou', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue18984'
versions = ['Python 3.4']

[tim-one]

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 ;-)

[pitrou]

Hmm.. I'm getting the following failures:

======================================================================
FAIL: test_4_joining_across_fork_in_worker_thread (test.test_threading.ThreadJoinOnShutdown)
----------------------------------------------------------------------

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'

======================================================================
FAIL: test_5_clear_waiter_locks_to_avoid_crash (test.test_threading.ThreadJoinOnShutdown)
----------------------------------------------------------------------

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'

[pitrou]

Looks like it's mostly the tests trying to check an implementation detail that doesn't exist anymore :-)

[tim-one]

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.

[pitrou]

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.

[python-dev]

New changeset aff959a3ba13 by Tim Peters in branch 'default':
bpo-18984: Remove ._stopped Event from Thread internals.
http://hg.python.org/cpython/rev/aff959a3ba13

[tim-one]

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.

[tim-one]

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

http://buildbot.python.org/all/builders/x86%20Ubuntu%20Shared%203.x/builds/8581/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 :-(

[pitrou]

Here is a fixed patch (works here, at least :-)).

[tim-one]

Well - I remain baffled, but am grateful for the patch - thanks :-)

[pitrou]

I suppose we can close this issue now?

[tim-one]

Yes - and I just closed 18808 :-)