gh-111506: Implement Py_REFCNT() as opaque function call

[vstinner]

In the limited C API version 3.13, the Py_REFCNT() function is now implemented as an opaque function call.

• Issue: --disable-gil builds are not compatible with the limited API #111506

[vstinner]

See also discussion #112553

[vstinner]

With this change, all functions reading/setting a Python object reference count go through opaque functions calls in the limited C API version 3.13 or newer:

• Py_INCREF(), Py_XINCREF()
• Py_NewRef(), Py_XNewRef()
• Py_DECREF(), Py_XDECREF()
• Py_CLEAR()
• Py_REFCNT()
• Py_SET_REFCNT()

So implementation details such as immortal objects (PEP 638) and free threading (PEP 703) "deferred reference counting" are no longer leaked at the ABI level.

[encukou]

I don't think Py_REFCNT should be added to the limited API.
What are the use cases for it?

[vstinner]

I don't think Py_REFCNT should be added to the limited API.

The Py_REFCNT() macro (now it's a static inline function) is part of the limited C API since Python 3.2 (PEP 384).

This change only fix its implementation to keep the backward compatibility at the API level. Currently, . You can build existing C extensions without having to change their C code. You just have to target the limited C API version 3.13 if you want to support the free threading build.

Currently, building Modules/xxlimited.c with the limited C API of a free threading build fails with:

./Include/object.h:297:22: error: implicit declaration of function '_Py_atomic_load_uint32_relaxed' [-Werror=implicit-function-declaration] ./Include/object.h:301:25: error: implicit declaration of function '_Py_atomic_load_ssize_relaxed' [-Werror=implicit-function-declaration] 

You can test locally with this change:

diff --git a/Modules/xxlimited.c b/Modules/xxlimited.c index 19f61216255..6d1ced8fb4a 100644 --- a/Modules/xxlimited.c +++ b/Modules/xxlimited.c @@ -62,14 +62,7 @@ pass */ -#ifndef _MSC_VER -#include "pyconfig.h" // Py_GIL_DISABLED -#endif - -#ifndef Py_GIL_DISABLED -// Need limited C API version 3.12 for Py_MOD_PER_INTERPRETER_GIL_SUPPORTED -#define Py_LIMITED_API 0x030c0000 -#endif +#define Py_LIMITED_API 0x030d0000 #include "Python.h" #include <string.h> 

[encukou]

Oh, I see. Py_REFCNT is not included in the limited API manifest, which is a bug (but a very low priority one I guess).

So, we're removing ob_refcnt (which is also specified in PEP 384), but keeping Py_REFCNT?

Is there a reason to name the function _Py_GetRefcnt, rather than keep the name people should use -- Py_REFCNT?

[vstinner]

So, we're removing ob_refcnt (which is also specified in PEP 384), but keeping Py_REFCNT?

This PR doesn't change PyObject.ob_refcnt. We might need to change it to fully implement free threading, but it's wider than the scope of this small change which is backward compatible (EDIT: compatible!).

Is there a reason to name the function _Py_GetRefcnt, rather than keep the name people should use -- Py_REFCNT?

I don't think that a regular function and a static inline functions can have the same name. With macros, it's possible to reuse the same name, but I don't want to use a macro here.

Py_INCREF() and Py_SET_REFCNT() have a similiar design: call an opaque function which has a different name and is only exposed as ABI only in the stable ABI.

[encukou]

I don't think that a regular function and a static inline functions can have the same name.

Please discuss at capi-workgroup/api-evolution#18, which has an example of how to do this.

With macros, it's possible to reuse the same name, but I don't want to use a macro here.

Not even the simple alias macro, #define Py_Foo _Py_Foo_impl?

Py_INCREF() and Py_SET_REFCNT() have a similiar design: call an opaque function which has a different name and is only exposed as ABI only in the stable ABI.

Yeah, that's the bug I was talking about. It's guideline issue 18 again: “All functions should be exported as proper symbols, so they are usable without a C preprocessor/parser.”

With this PR, Py_REFCNT is only usable in C. I've long thought that this is bad, but now I think that it should be more than personal preference, so, unlike in October, I'll block PRs on it.

[vstinner]

Apparently, there is a discussion/disagreement on the API/name in the ABI. I prefer to close the PR until a decision is taken.