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-108014: Add Py_IsFinalizing() function by vstinner · Pull Request #108032 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Aug 16, 2023 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • Issue: _Py_IsFinalizing has been removed, meaning C/C++ threads cannot avoid termination during shutdown #108014

📚 Documentation preview 📚: https://cpython-previews--108032.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
pythongh-108014: Add Py_IsFinalizing() function
e03b437
@vstinner
vstinner requested review from 1st1, asvetlov, gvanrossum, kumaraditya303 and willingc as code owners August 16, 2023 15:19
@bedevere-bot bedevere-bot mentioned this pull request Aug 16, 2023
_Py_IsFinalizing has been removed, meaning C/C++ threads cannot avoid termination during shutdown #108014
Closed
2 tasks done
@bedevere-bot bedevere-bot added the awaiting core review label Aug 16, 2023
@vstinner vstinner added topic-C-API and removed awaiting core review labels Aug 16, 2023
@vstinner

vstinner commented Aug 16, 2023

Copy link
Copy Markdown
Member Author

@ericsnowcurrently: Oh, that's a "feature" PR, so I will need a formal approval to merge it :-)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner mentioned this pull request Aug 16, 2023
Add Py_IsFinalizing() python/pythoncapi-compat#66
Merged
@vstinner

vstinner commented Aug 16, 2023

Copy link
Copy Markdown
Member Author

I prepared python/pythoncapi-compat#66 to add the function to the pythoncapi-compat project.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gvanrossum
gvanrossum removed their request for review August 16, 2023 18:05
ericsnowcurrently
ericsnowcurrently reviewed Aug 16, 2023
View reviewed changes
Comment thread Doc/c-api/init.rst Outdated
Comment on lines +378 to +379
Return non-zero if the Python interpreter is :term:`shutting down
<interpreter shutdown>`, return 0 otherwise.

@ericsnowcurrently ericsnowcurrently Aug 16, 2023

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.

Is this referring to the runtime (AKA main interpreter) or to the current interpreter? I'd expect it to be the current interpreter.

Would it make sense to explicitly pass an interpreter and use NULL to mean the current one (or main)?

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner Aug 16, 2023

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.

I copied the documentation of the sys.is_finalizing() function. If you want a different feature (pass an interpreter), I would prefer to have a different function.

It's set by Py_Finalize() which finalizes the main interpreter: other sub-interpreters should be deleted before explicitly, no?

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner Aug 17, 2023

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.

I updated the doc to specify that we are talking about the main interpreter.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
serhiy-storchaka
serhiy-storchaka reviewed Aug 17, 2023
View reviewed changes
Comment thread Doc/c-api/init.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/c-api/init.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@kumaraditya303
kumaraditya303 removed their request for review August 17, 2023 06:08
vstinner added 2 commits August 17, 2023 17:48
@vstinner
true/false
f769755
@vstinner
Specify *main* interpreter
9bd258f
serhiy-storchaka
serhiy-storchaka approved these changes Aug 17, 2023
View reviewed changes

@serhiy-storchaka serhiy-storchaka 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.

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 Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.13.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/C API/2023-08-16-17-16-19.gh-issue-108014.wXN3CF.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added the awaiting merge label Aug 17, 2023
vstinner and others added 2 commits August 17, 2023 22:01
@vstinner @serhiy-storchaka
Update Misc/NEWS.d/next/C API/2023-08-16-17-16-19.gh-issue-108014.wXN…
fe6216c 
…3CF.rst

Co-authored-by: Serhiy Storchaka <[email protected]>
@vstinner @serhiy-storchaka
Update Doc/whatsnew/3.13.rst
fa76703 
Co-authored-by: Serhiy Storchaka <[email protected]>
@vstinner
vstinner merged commit 3ff5ef2 into python:main Aug 18, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Aug 18, 2023
@vstinner
vstinner deleted the is_finalizing branch August 18, 2023 10:34
@vstinner

vstinner commented Aug 18, 2023

Copy link
Copy Markdown
Member Author

Thanks for the review @ericsnowcurrently and @serhiy-storchaka!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner mentioned this pull request Aug 29, 2023
C API: Remove private C API functions (move them to the internal C API) #106320
Closed
@vstinner vstinner mentioned this pull request Sep 25, 2023
test_concurrent_futures.test_shutdown: test_interpreter_shutdown() fails randomly (race condition) #109047
Closed
@vstinner

vstinner commented Oct 5, 2023

Copy link
Copy Markdown
Member Author

Follow-up: @wjakob asks to add this new function to the limited C API version 3.13.
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ericsnowcurrently ericsnowcurrently mentioned this pull request Oct 6, 2023
The Word "Finalizing" in C-API Function Names is Sometimes Misleading #110490
Open
@tacaswell tacaswell mentioned this pull request Apr 30, 2024
[Python] _Py_IsFinalizing will be removed in Python 3.13 apache/arrow#41475
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericsnowcurrently ericsnowcurrently ericsnowcurrently left review comments
@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
@willingc willingc Awaiting requested review from willingc willingc is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

topic-C-API

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 @serhiy-storchaka @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.