Skip to content
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

gh-108014: Add Py_IsFinalizing() function #108032

Merged
merged 5 commits into from
Aug 18, 2023
Merged

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Aug 16, 2023

@vstinner
Copy link
Member Author

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

@vstinner
Copy link
Member Author

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

@gvanrossum gvanrossum removed their request for review August 16, 2023 18:05
Comment on lines 378 to 379
Return non-zero if the Python interpreter is :term:`shutting down
<interpreter shutdown>`, return 0 otherwise.
Copy link
Member

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)?

Copy link
Member Author

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?

Copy link
Member Author

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.

Doc/c-api/init.rst Show resolved Hide resolved
Doc/c-api/init.rst Outdated Show resolved Hide resolved
@kumaraditya303 kumaraditya303 removed their request for review August 17, 2023 06:08
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

Doc/c-api/init.rst Show resolved Hide resolved
Doc/whatsnew/3.13.rst Outdated Show resolved Hide resolved
@vstinner vstinner merged commit 3ff5ef2 into python:main Aug 18, 2023
17 checks passed
@vstinner vstinner deleted the is_finalizing branch August 18, 2023 10:34
@vstinner
Copy link
Member Author

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

@vstinner
Copy link
Member Author

vstinner commented Oct 5, 2023

Follow-up: @wjakob asks to add this new function to the limited C API version 3.13.

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.

4 participants