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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-110397: Add Py_IsFinalizing() to the stable ABI #110441

Merged
merged 1 commit into from
Oct 7, 2023

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Oct 5, 2023

@vstinner
Copy link
Member Author

vstinner commented Oct 5, 2023

@ericsnowcurrently @serhiy-storchaka: @wjakob explains that reimplementing this function with the existing limited C API is likely to crash if calling during or after Python finalization. See: #110397 (comment) Would you be ok to add Py_IsFinalizing() directly to the limited C API version 3.13, knowing that this function was added to Python 3.13?

This Py_IsFinalizing() function looks special and it's hard to workaround the lack of this API with the stable ABI.

@erlend-aasland
Copy link
Contributor

I'm fine with adding it to the Stable ABI, FWIW.

@ericsnowcurrently
Copy link
Member

FWIW, before we add the function to the stable ABI, it may be worth being clear about what "finalizing" means here. Consequently, we should consider actually making the Py_IsFinalizing() behavior match its name or change the name.

I've written up my thoughts in detail in gh-110490.

I realize that the stable ABI (and limited API) are specifically focused on memory layout, function signatures, and symbols (so it isn't critical if we change behavior later). Yet, there is also an implication of semantic stability (within reason) that we should avoid setting ourselves up to later break with this function. On the other hand, a different name might make sense, so we should avoid locking the symbol into the ABI prematurely.

@vstinner
Copy link
Member Author

vstinner commented Oct 6, 2023

FWIW, before we add the function to the stable ABI, it may be worth being clear about what "finalizing" means here.

This API is just the same as sys.is_finalizing() which is already part of the "stable" Python API.

@vstinner vstinner marked this pull request as ready for review October 7, 2023 15:57
@vstinner vstinner requested review from a team and encukou as code owners October 7, 2023 15:57
@vstinner vstinner merged commit 64f158e into python:main Oct 7, 2023
28 checks passed
@vstinner vstinner deleted the is_finalizing_stable_abi branch October 7, 2023 15:59
@vstinner
Copy link
Member Author

vstinner commented Oct 7, 2023

@ericsnowcurrently: I merged this PR since I'm feeling responsible of breaking applications relying on Python 3.12 private _Py_IsFinalizing() API. This C API exposes Python sys.is_finalizing() API which exists since Python 3.5.

For sure, there is always room for enhancement. It's a very complex topic, I wrote articles about it and I'm keeping notes about it: Python Finalization.

While it's possible to enhance the API, make finalization more reliable, reduce the risk of crashes, IMO we need "something" for people who are already affected by known issues. @wjakob elaborated issues that this API is solving in #110397 (comment) and just for that, I think that such API is worth it.

Python 3.13 didn't get an alpha1 release. We still have time until October 2024 to refine the documentation, the function name, come with a better API, etc. There is a non-zero risk that we will keep the name and so we will be good :-)

@bedevere-bot
Copy link

鈿狅笍鈿狅笍鈿狅笍 Buildbot failure 鈿狅笍鈿狅笍鈿狅笍

Hi! The buildbot s390x Fedora Clang Installed 3.x has failed when building commit 64f158e.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/531/builds/4639) and take a look at the build logs.
  4. Check if the failure is related to this commit (64f158e) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/531/builds/4639

Failed tests:

  • test_subprocess

Failed subtests:

  • test_pipesize_default - test.test_subprocess.ProcessTestCaseNoPoll.test_pipesize_default
  • test_pipesizes - test.test_subprocess.ProcessTestCaseNoPoll.test_pipesizes
  • test_pipesize_default - test.test_subprocess.ProcessTestCase.test_pipesize_default
  • test_pipesizes - test.test_subprocess.ProcessTestCase.test_pipesizes

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang-installed/build/target/lib/python3.13/test/test_subprocess.py", line 769, in test_pipesize_default
    self.assertEqual(
AssertionError: 65536 != 8192


Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang-installed/build/target/lib/python3.13/test/test_subprocess.py", line 727, in test_pipesizes
    p = subprocess.Popen(
        ^^^^^^^^^^^^^^^^^
  File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang-installed/build/target/lib/python3.13/subprocess.py", line 992, in __init__
    errread, errwrite) = self._get_handles(stdin, stdout, stderr)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang-installed/build/target/lib/python3.13/subprocess.py", line 1731, in _get_handles
    fcntl.fcntl(errwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
PermissionError: [Errno 1] Operation not permitted

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.

None yet

5 participants