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

Proposal to add Py_IsFinalizing() to the limited API/stable ABI #110397

Closed
wjakob opened this issue Oct 5, 2023 · 12 comments
Closed

Proposal to add Py_IsFinalizing() to the limited API/stable ABI #110397

wjakob opened this issue Oct 5, 2023 · 12 comments
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@wjakob
Copy link
Contributor

wjakob commented Oct 5, 2023

Feature or enhancement

Proposal:

Bigger Python extension projects sometimes need to check whether the interpreter is in the process of shutting down to determine if certain operations may be safely executed (PyEval_RestoreThread, Py_DECREF, etc.). Making a mistake here would cause a segfault, and the functions Py_IsFinalizing (previously _Py_IsFinalizing) are important to keep that from happening.

Just recently, this function was deleted, then re-added to the public API (#108014). However, it is not part of the limited API and therefore cannot be used in extension modules targeting the stable ABI.

The purpose of this ticket is to start a discussion on whether this function could be exposed as part of these longer-term stable interfaces.

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

@wjakob wjakob added the type-feature A feature request or enhancement label Oct 5, 2023
@wjakob
Copy link
Contributor Author

wjakob commented Oct 5, 2023

@wjakob wjakob changed the title Proposal to add Py_IsFinalizing to the limited API/stable ABI Proposal to add Py_IsFinalizing() to the limited API/stable ABI Oct 5, 2023
@vstinner
Copy link
Member

vstinner commented Oct 5, 2023

I added Py_IsFinalizing() the public C API. I would be fine with adding it the limited C API version 3.13.

Just recently, this function was deleted, then re-added to the public API (#108014). However, it is not part of the limited API and therefore cannot be used in extension modules targeting the stable ABI.

What's your usage of the stable ABI? I'm now curious :-)

@wjakob
Copy link
Contributor Author

wjakob commented Oct 5, 2023

I added Py_IsFinalizing() the public C API. I would be fine with adding it the limited C API version 3.13.

That would be great!

What's your usage of the stable ABI? I'm now curious :-)

I've been developing nanobind as a successor project (at least for my needs) to pybind11. As of Python 3.12, nanobind uses the limited API and so can be used to create stable-ABI bindings of large C++ projects. My hope is that many people will use it to build such stable ABI extensions in the future.

But such larger projects tend to sprinkle around a few calls to Py_IsFinalizing() or _Py_IsFinalizing(), so that would be nice to have part of the limited API as well. It's a bummer I did not think of raising this point before Python 3.12 was released -- do you see any chance of sneaking such a change into a patch release of 3.12 as well?

@erlend-aasland
Copy link
Contributor

It's a bummer I did not think of raising this point before Python 3.12 was released -- do you see any chance of sneaking such a change into a patch release of 3.12 as well?

I'm not sure we've added to the Limited API in patch releases before. My gut feeling is that we cannot add it in a 3.12 patch release. Victor, what do you think?

I'm fine with adding Py_IsFinalizing() to the Limited API, though :) Let's start with main.

@vstinner
Copy link
Member

vstinner commented Oct 5, 2023

e. My gut feeling is that we cannot add it in a 3.12 patch release. Victor, what do you think?

We cannot add functions to the stable ABI in a minor release, the ABI must not change. That's why it's called stable :-)

@erlend-aasland
Copy link
Contributor

For some reason, I assumed it already was in the Stable ABI and not in the Limited API ☕☕

@vstinner
Copy link
Member

vstinner commented Oct 5, 2023

It's a bummer I did not think of raising this point before Python 3.12 was released -- do you see any chance of sneaking such a change into a patch release of 3.12 as well?

You can reimplement the function in a few lines of C code. Example without error handling with 3 function calls:

static int myPy_IsFinalizing(void)
{
    PyObject *sys = PyImport_ImportModule("sys");
    assert(sys != NULL);

    PyObject *res = PyObject_CallMethod(sys, "is_finalizing", NULL);
    Py_DECREF(sys);
    assert(res != NULL);

    int is_finalizing = PyObject_IsTrue(res);
    Py_DECREF(res);
    assert(is_finalizing != -1);

    return is_finalizing;
}

pythoncapi-compat reimplements it on Python 3.2-3.12 with private functions:

// gh-108014 added Py_IsFinalizing() to Python 3.13.0a1
// bpo-1856 added _Py_Finalizing to Python 3.2.1b1.
// _Py_IsFinalizing() was added to PyPy 7.3.0.
#if (0x030201B1 <= PY_VERSION_HEX && PY_VERSION_HEX < 0x030D00A1) \
        && (!defined(PYPY_VERSION_NUM) || PYPY_VERSION_NUM >= 0x7030000)
static inline int Py_IsFinalizing(void)
{
#if PY_VERSION_HEX >= 0x030700A1
    // _Py_IsFinalizing() was added to Python 3.7.0a1.
    return _Py_IsFinalizing();
#else
    return (_Py_Finalizing != NULL);
#endif
}
#endif

@vstinner
Copy link
Member

vstinner commented Oct 5, 2023

There is a big work-in-progress project in Python 3.13 to convert some private C functions to public functions (excluded from the limited C API).

We can consider adding some of them to the limited C API, on demand, like you do.

@wjakob
Copy link
Contributor Author

wjakob commented Oct 5, 2023

Thank you for the suggestion @vstinner. This works while Python is still sufficiently alive, but I suspect that your proposed implementation of myPy_IsFinalizing will fail once the interpreter has fully shut down.

That could actually happen in a C++ context: destructors of global data structures, instances of classes stored in TLS objects (C++11-style thread_local). These will run when a thread or the entire process exits. _Py_IsFinalizing() still worked under such circumstances and returned true, while PyImport_ImportModule seems like it would cause a segfault.

@vstinner
Copy link
Member

vstinner commented Oct 5, 2023

Oh wait, now I'm confused.

  • In Python <= 3.12, there was a private _Py_IsFinalizing() function.
  • In Python 3.13, there is a new public Py_IsFinalizing() function, but it's excluded from the limited C API.

I thought that I added Py_IsFinalizing() to Python 3.12.

Usually, I prefer to wait one Python release before adding a function to the stable ABI. If we mess up with a public C API, fixing it is challenging. If we mess up a stable ABI, it's very more annoying to fix.

I suggest to wait until Python 3.14 to add this function to the stable ABI.

@wjakob: Anyway, you need to reimplement Py_IsFinalizing() in Python <= 3.12.

Waiting one version before adding a function to the stable ABI is not a strict rule. Maybe new functions are added directly to the public C API and the stable ABI at the same time.

@vstinner
Copy link
Member

vstinner commented Oct 5, 2023

I've been developing nanobind as a successor project (at least for my needs) to pybind11. As of Python 3.12, nanobind uses the limited API and so can be used to create stable-ABI bindings of large C++ projects. My hope is that many people will use it to build such stable ABI extensions in the future.

That's great!

I created PR gh-110441 to implement this change. I will try to get the opinion of other people about this change.

@vstinner
Copy link
Member

vstinner commented Oct 7, 2023

Implemented as commit 64f158e

But see @ericsnowcurrently's concerns about this API in issue gh-110490. Let's continue the discussion there.

Thanks @wjakob for your feedback on your usage of this API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants