Skip to content

[3.7] bpo-36402: Fix threading._shutdown() race condition (GH-13948) #14034

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

Closed

Conversation

miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Jun 12, 2019

Fix a race condition at Python shutdown when waiting for threads.
Wait until the Python thread state of all non-daemon threads get
deleted (join all non-daemon threads), rather than just wait until
Python threads complete.

  • Add threading._shutdown_locks: set of Thread._tstate_lock locks
    of non-daemon threads used by _shutdown() to wait until all Python
    thread states get deleted. See Thread._set_tstate_lock().
  • Add also threading._shutdown_locks_lock to protect access to
    threading._shutdown_locks.
  • Add test_finalization_shutdown() test.
    (cherry picked from commit 468e5fe)

Co-authored-by: Victor Stinner [email protected]

https://bugs.python.org/issue36402

Fix a race condition at Python shutdown when waiting for threads.
Wait until the Python thread state of all non-daemon threads get
deleted (join all non-daemon threads), rather than just wait until
Python threads complete.

* Add threading._shutdown_locks: set of Thread._tstate_lock locks
  of non-daemon threads used by _shutdown() to wait until all Python
  thread states get deleted. See Thread._set_tstate_lock().
* Add also threading._shutdown_locks_lock to protect access to
  threading._shutdown_locks.
* Add test_finalization_shutdown() test.
(cherry picked from commit 468e5fe)

Co-authored-by: Victor Stinner <[email protected]>
@vstinner
Copy link
Member

I prefer to wait to see how buildbots like PR #13948 merged into master, before merging the backport to 3.7.

@miss-islington
Copy link
Contributor Author

@vstinner: Status check is done, and it's a success ✅ .

1 similar comment
@miss-islington
Copy link
Contributor Author

@vstinner: Status check is done, and it's a success ✅ .

@vstinner
Copy link
Member

My change introduced a giant regression! https://bugs.python.org/issue37265 ... the fix is stupid :-) #14047. I will backport the change manually using my fix.

@vstinner vstinner closed this Jun 13, 2019
@miss-islington miss-islington deleted the backport-468e5fe-3.7 branch June 13, 2019 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants