Skip to content

gh-128639: Don't assume one thread in subinterpreter finalization with fixed daemon thread support #134606

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ZeroIntensity
Copy link
Member

@ZeroIntensity ZeroIntensity commented May 24, 2025

This is a reapplication of #128640, with daemon thread hanging fixed.

The key part that I was missing was this:

for (PyThreadState *p = list; p != NULL; p = p->next) {
    _PyThreadState_SetShuttingDown(p);
}

My fix predated these lines, so it didn't make it in originally.

The reason this only failed on the iOS buildbots was because they're single-process. Out of random choice, I picked 100 seconds as the sleep time, but since the process would exit before that 100 seconds was over on all the other buildbots, there wasn't any problem. On iOS, the test suite continued to run in the same process after the 100 seconds were up, so the daemon threads would start trying to run again under a deallocated interpreter, causing all sorts of weird crashes.

@bedevere-bot

This comment was marked as outdated.

@ZeroIntensity ZeroIntensity added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 24, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit a569355 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134606%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 24, 2025
@ZeroIntensity

This comment was marked as outdated.

@ZeroIntensity

This comment was marked as outdated.

@bedevere-bot

This comment was marked as outdated.

@ZeroIntensity
Copy link
Member Author

!buildbot refleak

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit f85a291 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134606%2Fmerge

The command will test the builders whose names match following regular expression: refleak

The builders matched are:

  • aarch64 Fedora Rawhide NoGIL refleaks PR
  • AMD64 Fedora Rawhide NoGIL refleaks PR
  • PPC64LE RHEL8 Refleaks PR
  • PPC64LE Fedora Rawhide NoGIL refleaks PR
  • PPC64LE CentOS9 Refleaks PR
  • s390x Fedora Rawhide NoGIL refleaks PR
  • AMD64 Fedora Stable Refleaks PR
  • AMD64 RHEL8 Refleaks PR
  • AMD64 Windows11 Refleaks PR
  • aarch64 Fedora Stable Refleaks PR
  • AMD64 Fedora Rawhide Refleaks PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • aarch64 Fedora Rawhide Refleaks PR
  • aarch64 RHEL8 Refleaks PR
  • s390x RHEL8 Refleaks PR
  • s390x Fedora Rawhide Refleaks PR
  • AMD64 FreeBSD Refleaks PR
  • aarch64 CentOS9 Refleaks PR
  • AMD64 CentOS9 NoGIL Refleaks PR
  • PPC64LE Fedora Stable Refleaks PR
  • PPC64LE Fedora Rawhide Refleaks PR
  • s390x RHEL9 Refleaks PR
  • AMD64 CentOS9 Refleaks PR
  • s390x Fedora Stable Refleaks PR

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.

2 participants