Skip to content
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

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

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 vstinner@redhat.com

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 <vstinner@redhat.com>
@vstinner
Copy link
Member

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

@miss-islington
Copy link
Contributor Author

@vstinner: Status check is done, and it's a failure ❌ .

@gpshead
Copy link
Member

gpshead commented Jun 13, 2019

the build failure on the azure CI is macOS hanging within
test_importlib/test_locks.py", line 84 in run_deadlock_avoidance_test

thoughts on if this is spurious or related?

regardless, i think your change looks good.

@vstinner
Copy link
Member

It seems like macOS of Azure Pipelines is sick: https://bugs.python.org/issue37245

@vstinner
Copy link
Member

me:

regardless, i think your change looks good.

Gregory:

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

It also LGTM but I was right to wait for buildbots: I 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.8 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.

5 participants