Skip to content

Remove .stopped Event from Thread internals #63184

Closed
Closed
Remove .stopped Event from Thread internals#63184
Assignees
tim-one
Labels
performancePerformance or resource usagestdlibStandard Library Python modules in the Lib/ directory

Description

@tim-one
tim-one
opened on Sep 8, 2013
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']

    Activity

    1. tim-one commented on Sep 8, 2013

      @tim-one
      tim-one
      on Sep 8, 2013
      MemberAuthor

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

    2. tim-one
      self-assigned this
      on Sep 8, 2013
    3. tim-one
      added
      stdlibStandard Library Python modules in the Lib/ directory
      on Sep 8, 2013

    4. pitrou commented on Sep 8, 2013

      @pitrou
      pitrou
      on Sep 8, 2013
      Member

      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'

    5. pitrou commented on Sep 8, 2013

      @pitrou
      pitrou
      on Sep 8, 2013
      Member

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

    6. tim-one commented on Sep 8, 2013

      @tim-one
      tim-one
      on Sep 8, 2013
      MemberAuthor

      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.

    7. pitrou commented on Sep 8, 2013

      @pitrou
      pitrou
      on Sep 8, 2013
      Member

      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.

    8. pitrou
      added
      performancePerformance or resource usage
      on Sep 8, 2013

    9. python-dev commented on Sep 8, 2013

      @python-dev
      python-devmannequin
      on Sep 8, 2013
      Mannequin

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

    10. tim-one commented on Sep 8, 2013

      @tim-one
      tim-one
      on Sep 8, 2013
      MemberAuthor

      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.

    11. tim-one commented on Sep 9, 2013

      @tim-one
      tim-one
      on Sep 9, 2013
      MemberAuthor

      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 :-(

    12. pitrou commented on Sep 9, 2013

      @pitrou
      pitrou
      on Sep 9, 2013
      Member

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

    13. tim-one commented on Sep 9, 2013

      @tim-one
      tim-one
      on Sep 9, 2013
      MemberAuthor

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

    14. pitrou commented on Sep 9, 2013

      @pitrou
      pitrou
      on Sep 9, 2013
      Member

      I suppose we can close this issue now?

    15. tim-one commented on Sep 9, 2013

      @tim-one
      tim-one
      on Sep 9, 2013
      MemberAuthor

      Yes - and I just closed 18808 :-)

    16. tim-one
      closed this as completedon Sep 9, 2013
    17. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    performancePerformance or resource usagestdlibStandard Library Python modules in the Lib/ directory

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions