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-39877: _PyRuntimeState.finalizing becomes atomic by vstinner · Pull Request #18816 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Mar 6, 2020 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Convert _PyRuntimeState.finalizing field to an atomic variable:

  • Rename it to _finalizing
  • Change its type to _Py_atomic_address
  • Add _PyRuntimeState_GetFinalizing() and _PyRuntimeState_SetFinalizing()
    functions
  • Remove _Py_CURRENTLY_FINALIZING() function: replace it with testing
    directly _PyRuntimeState_GetFinalizing() value

Convert _PyRuntimeState_GetThreadState() to static inline function.

https://bugs.python.org/issue39877

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
bpo-39877: _PyRuntimeState.finalizing becomes atomic
699e914 
Convert _PyRuntimeState.finalizing field to an atomic variable:

* Rename it to _finalizing
* Change its type to _Py_atomic_address
* Add _PyRuntimeState_GetFinalizing() and _PyRuntimeState_SetFinalizing()
  functions
* Remove _Py_CURRENTLY_FINALIZING() function: replace it with testing
  directly _PyRuntimeState_GetFinalizing() value

Convert _PyRuntimeState_GetThreadState() to static inline function.
@vstinner
vstinner requested a review from pitrou March 6, 2020 21:59
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Mar 6, 2020
@bedevere-bot bedevere-bot added the awaiting core review label Mar 6, 2020
@vstinner vstinner added the skip news label Mar 6, 2020
@vstinner

vstinner commented Mar 6, 2020

Copy link
Copy Markdown
Member Author

This PR prepares the code for PR #18811 which does no longer protect _PyRuntimeState.finalizing with the GIL. See discussion: #18811 (comment)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner mentioned this pull request Mar 6, 2020
bpo-39877: Fix PyEval_RestoreThread() for daemon threads #18811
Merged
pitrou
pitrou approved these changes Mar 6, 2020
View reviewed changes

@pitrou pitrou 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, but need to fix that pesky MSVC failure.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/internal/pycore_pystate.h Outdated

static inline void
_PyRuntimeState_SetFinalizing(_PyRuntimeState *runtime, PyThreadState *tstate) {
return _Py_atomic_store_relaxed(&runtime->_finalizing, (uintptr_t)tstate);

@pitrou pitrou Mar 6, 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.

I think the return is confusing MSVC (see CI failures).

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 Mar 6, 2020
@vstinner
Fix Windows build
c5926d9
@vstinner
vstinner merged commit 7b3c252 into python:master Mar 6, 2020
@bedevere-bot bedevere-bot removed the awaiting merge label Mar 6, 2020
@vstinner
vstinner deleted the atomic_finalizing branch March 6, 2020 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitrou pitrou pitrou approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

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.

4 participants

@vstinner @pitrou @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.