GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-104341: Wait Completely at threading._shutdown() by ericsnowcurrently · Pull Request #104672 · python/cpython

Conversation

@ericsnowcurrently

@ericsnowcurrently ericsnowcurrently commented May 19, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

(This should solve the semi-frequent buildbot failures we've been seeing.)

In Py_EndInterpreter() we almost immediately wait for all non-daemon Python threads to finish, by calling theading._shutdown(). However, with a per-interpreter GIL there's a race because threading._shutdown() doesn't wait long enough. This change fixes that by making sure _PyThreadState_DeleteCurrent() completely finishes before releasing the lock on which threading._shutdown() depends.

The gist of it is that we release that lock (and clean it up) later than we would normally be able to, by doing the cleanup in another thread via a "pending" call.

This isn't an ideal solution, but other approaches I tried involved more invasive changes, which I'd like to avoid this close to the beta 1 release.

  • Issue: test__xxsubinterpreters is Occasionally Crashing #104341

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@ericsnowcurrently ericsnowcurrently added the skip news label May 19, 2023
@bedevere-bot bedevere-bot added the awaiting core review label May 19, 2023
@bedevere-bot bedevere-bot mentioned this pull request May 19, 2023
test__xxsubinterpreters is Occasionally Crashing #104341
Closed
@ericsnowcurrently

ericsnowcurrently commented May 19, 2023

Copy link
Copy Markdown
Member Author

I plan on revisiting this problem, to introduce a better long-term solution. However, that probably won't be in 3.12.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ericsnowcurrently
ericsnowcurrently marked this pull request as draft May 19, 2023 22:39
@bedevere-bot bedevere-bot removed the awaiting core review label May 19, 2023
@ericsnowcurrently ericsnowcurrently mentioned this pull request May 23, 2023
gh-104812: Run Pending Calls in any Thread #104813
Merged
ericsnowcurrently added 3 commits May 23, 2023 14:58
@ericsnowcurrently
Release the shutdown lock *after* releasing the GIL.
99ffe4b
@ericsnowcurrently
Respect the GIL.
e5b6cb4
@ericsnowcurrently
Bump the number of pending calls.
8b42a15
@ericsnowcurrently
ericsnowcurrently force-pushed the fix-threading-module-shutdown-simpler branch from 6c79a9b to 8b42a15 Compare May 23, 2023 21:00
@ericsnowcurrently
ericsnowcurrently marked this pull request as ready for review May 23, 2023 21:01
@bedevere-bot bedevere-bot added the awaiting core review label May 23, 2023
@markshannon

markshannon commented May 31, 2023

Copy link
Copy Markdown
Member

Ae you still going with this, or has it been superseded?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ericsnowcurrently

ericsnowcurrently commented May 31, 2023

Copy link
Copy Markdown
Member Author

It's superseded for the moment. I'm likely to revisit this for 3.13.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ericsnowcurrently ericsnowcurrently closed this May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting core review skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@ericsnowcurrently @markshannon @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.