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 — bpo-39511: PyThreadState_Clear() calls on_delete by vstinner · Pull Request #18296 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Jan 31, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

PyThreadState.on_delete is a callback used to notify Python when a
thread completes. _thread._set_sentinel() function creates a lock
which is released when the thread completes. It sets on_delete
callback to the internal release_sentinel() function. This lock is
known as Threading._tstate_lock in the threading module.

The release_sentinel() function uses the Python C API. The problem is
that on_delete is called late in the Python finalization, when the C
API is no longer fully working.

The PyThreadState_Clear() function is now responsible to call
PyThreadState.on_delete callback. Previously, PyThreadState_Delete()
was responsible for that.

The release_sentinel() function is now called when the C API is still
fully working.

https://bugs.python.org/issue39511

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jan 31, 2020
@bedevere-bot bedevere-bot added the awaiting core review label Jan 31, 2020
@vstinner

vstinner commented Jan 31, 2020

Copy link
Copy Markdown
Member Author

Eric snow @ericsnowcurrently already faced this issue when trying to fix https://bugs.python.org/issue36854 with his PR #13219. I managed to work around this issue by clearing the Python thread state later: commit 9da7430.

But to implement https://bugs.python.org/issue39511 correctly (clear singletons at exit), I have to fix this issue.
All reactions

Sorry, something went wrong.

Uh oh!

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

vstinner commented Jan 31, 2020

Copy link
Copy Markdown
Member Author

I decided to not document the change in What's New in Python 3.9 since I don't think that PyThreadState_Clear() should be called directly. By the way, I'm not sure why it's documented at all nor why it's a public function. For me, it belongs more to the internal C API.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ericsnowcurrently
ericsnowcurrently approved these changes Jan 31, 2020
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I only have one suggestion on the wording of the docs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/c-api/init.rst Outdated
Comment on lines 1052 to 1054

@ericsnowcurrently ericsnowcurrently Jan 31, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This usage of "responsible" could be confusing. Consider instead:

   This function now calls the :c:member:`PyThreadState.on_delete`
   callback.  Previously, that happened in :c:func:`PyThreadState_Delete`.

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner Feb 1, 2020

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I updated the doc.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jan 31, 2020
@vstinner
bpo-39511: PyThreadState_Clear() calls on_delete
5e39ae9 
PyThreadState.on_delete is a callback used to notify Python when a
thread completes. _thread._set_sentinel() function creates a lock
which is released when the thread completes. It sets on_delete
callback to the internal release_sentinel() function. This lock is
known as Threading._tstate_lock in the threading module.

The release_sentinel() function uses the Python C API. The problem is
that on_delete is called late in the Python finalization, when the C
API is no longer fully working.

The PyThreadState_Clear() function now calls the
PyThreadState.on_delete callback. Previously, that happened in
PyThreadState_Delete().

The release_sentinel() function is now called when the C API is still
fully working.
@vstinner
vstinner merged commit 4d96b46 into python:master Feb 1, 2020
@bedevere-bot bedevere-bot removed the awaiting merge label Feb 1, 2020
@vstinner
vstinner deleted the zapthreads branch February 1, 2020 01:30
@erlend-aasland erlend-aasland mentioned this pull request May 22, 2021
bpo-44032: Defer clearing last thread state until last GC has been run. #26285
Merged
@vstinner vstinner mentioned this pull request Apr 10, 2022
[subinterpreters] Per-interpreter singletons (None, True, False, etc.) #83692
Closed
@rohan-97 rohan-97 mentioned this pull request Dec 23, 2022
Mod Python causing segmentation faults when compiled with Python3.9 grisha/mod_python#122
Open
This was referenced May 13, 2023
test__xxsubinterpreters is Occasionally Crashing #104341
Closed
gh-104341: Fix threading Module Shutdown #104560
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericsnowcurrently ericsnowcurrently ericsnowcurrently approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

4 participants

@vstinner @ericsnowcurrently @the-knights-who-say-ni @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.