ABI compatibility between master and smart_holder branches

[rhaschke]

Hi @rwgk,
first of all: thanks for your continued efforts to merge smart_holder and master branches.
I have been using both branches in parallel for quite a while, having some modules built with official releases (master) and some building directly from smart_holder branch. This worked quite well so far, using PYBIND11_INTERNALS_VERSION=4 for both branches.

Recently, I discovered a segfault in this 9-month-old configuration, which turned out to be an ABI incompatibility. Fortunately, due to your efforts, the issue was already fixed in #5257. However, forwarding to this commit broke compatibility between master and smart_holder as the latter was fixed to PYBIND11_INTERNALS_VERSION=6 by default:

pybind11/include/pybind11/detail/internals.h

Lines 39 to 40 in 48f2527

	# if PYBIND11_VERSION_MAJOR >= 3
	# define PYBIND11_INTERNALS_VERSION 6

Only recently, the master branch was upgraded to PYBIND11_INTERNALS_VERSION=6 as well (#5512).
However, 3 days ago, you increased PYBIND11_VERSION_MAJOR -> 3 and PYBIND11_INTERNALS_VERSION -> 106, again breaking ABI compatibility:

pybind11/include/pybind11/detail/internals.h

Lines 40 to 41 in 7d37eb9

	# if PYBIND11_VERSION_MAJOR >= 3
	# define PYBIND11_INTERNALS_VERSION 106

Thus, currently we have only a single combination of master (#5512) and smart_holder (664876e) branches being compatible.
EDIT: One can explicitly enforce PYBIND11_INTERNALS_VERSION=6 on release versions >= 2.13.1.
The smart_holder branch, on the other hand, does not work with PYBIND11_INTERNALS_VERSION<6. This should be asserted:

• [smart_holder] Disable internals version 4 when including smart_holder.h #5525

---

What is really annoying about the current situation, is that we cannot detect ABI incompatibility anymore: Previously, I used the number of __builtins__ keys starting with __pybind11_internals_v as an indicator for the number of different ABI versions loaded:

versions = [k for k in __builtins__.__dict__.keys() if k.startswith("__pybind11_internals_v")]
assert len(versions) == 1, "Mixing incompatible pybind11 versions"

However, this doesn't work anymore since #5257 doesn't expose this key anymore. Now, I need to correctly interpret TypeError exceptions like these:

Unable to convert call argument '0' of type 'std::shared_ptr<moveit::core::RobotModel const>' to Python object
or
Unable to convert function return value to a Python type! The signature was
        (arg0: pymoveit_mtc.core.InterfaceState) -> planning_scene::PlanningScene

However, these also popup if the types are not exposed at all (instead of being hidden in some incompatible module).
Hence, my suggestion: Perform a sanity check when loading an extension module and throw a warning when multiple different ABI versions are mixed. Consider all PYBIND11_INTERNALS_VERSION >= 4. I hope, there is an easy way to do so.

[rwgk]

I'm confused TBH.

Trying to get a handle on it:

• I made the change introducing 106 as the smart_holder internals version (https://github.com/pybind/pybind11/pull/5519/files) in a rush, because I realized that we had a lingering conflict that had slipped my mind:

• Background: Unfortunately my attempt to merge the smart_holder branch into master ([Preview 3] Merge smart_holder into master (squash merge) #5339), around the end of August 2024, fell flat, because of complete radio silence from all potential reviewers. As a result, I left the branch on PYBIND11_INTERNALS_VERSION 6.

• In early September 2024, PR Added exception translator specific mutex used with try_translate_exceptions #5362 was merged, which included this change:

--- a/include/pybind11/detail/internals.h
+++ b/include/pybind11/detail/internals.h
@@ -39,7 +39,11 @@
 #    if PY_VERSION_HEX >= 0x030C0000 || defined(_MSC_VER)
 // Version bump for Python 3.12+, before first 3.12 beta release.
 // Version bump for MSVC piggy-backed on PR #4779. See comments there.
-#        define PYBIND11_INTERNALS_VERSION 5
+#        ifdef Py_GIL_DISABLED
+#            define PYBIND11_INTERNALS_VERSION 6
+#        else
+#            define PYBIND11_INTERNALS_VERSION 5
+#        endif
 #    else
 #        define PYBIND11_INTERNALS_VERSION 4
 #    endif

That introduced a PYBIND11_INTERNALS_VERSION clash with the smart_holder branch, but it slipped my mind. It didn't come to the forefront because PYBIND11_INTERNALS_VERSION 6 was only used for free-threaded Python.

• Then, when I merged Make PYBIND11_INTERNALS_VERSION 6 the default on all platforms. #5512 a few days ago, the conflict became apparent.

However:

• As of PR feat: enable type-safe interoperability between different independent Python/C++ bindings systems. #5296, we do have the "conduit feature" on both the master and the smart_holder branches, which means that PYBIND11_INTERNALS_VERSION isn't nearly as critical anymore as it was in the past. See the description of PR 5296 for details. That's why I was thinking that adopting 106 as the smart_holder internals version should be fine.

• I believe that, most likely, all you need to do is NOT override PYBIND11_INTERNALS_VERSION. Did you consider or try that already?

The only problematic situation I'd expect: If you have a base class in one extension module using internals version A, and a derived class in another extension module using internals version B. (Both extensions would have to either use master or smart_holder, you couldn't use a mix.)

All other cross-extension functionality should work based on the conduit feature. The only requirement is that the PYBIND11_PLATFORM_ABI_IDs need to match between the extensions, which is the case for current master and smart_holder.

[rhaschke]

• As of PR feat: enable type-safe interoperability between different independent Python/C++ bindings systems. #5296, we do have the "conduit feature" on both the master and the smart_holder branches, which means that PYBIND11_INTERNALS_VERSION isn't nearly as critical anymore as it was in the past. See the description of PR 5296 for details. That's why I was thinking that adopting 106 as the smart_holder internals version should be fine.

I didn't notice that. Cool! Sorry, for not intensely following the pybind11 development anymore. In most situations it just works.

• I believe that, most likely, all you need to do is NOT override PYBIND11_INTERNALS_VERSION. Did you consider or try that already?

Sure. That's what I tried first. However, when testing, I didn't yet considered rebuilding module A too, which was built against a 9-month old pybind11 version, not including the conduit feature yet. Only module B was built against the latest versions. This probably explains that I still didn't have interoperability. I will try rebuilding both modules now and report back.

[rhaschke]

Unfortunately, building module A with v2.13.1 and module B with HEAD of smart_holder branch still has compatibility issues.
As far as I understood, v2.13.1 was the first release to include the conduit feature, wasn't it?
Is there a way to check the ABI version of a built extension module?

The second exception in my original post, pops up when returning a C++ object exposed in module A (v2.13.1) through a function exposed in module B (smart_holder).
The first exception happens, when a C++ object (module A) is passed to a method of a trampoline class (deriving from a class exposed in module B).
I will try to come up with a minimal unit test for these during the coming week. Can you point me to corresponding unit tests of the conduit feature?

[rwgk]

Please see the PR #5512 description, in particular the part near the bottom. Copying here for easy reference:

ABI break: Extensions built with e.g. pybind11 2.13 will not interoperate with extensions built with pybind11 2.14.

Note for completeness: PR #5296 greatly reduced the impact of changing the PYBIND11_INTERNALS_VERSION (see PR description there). The ABI break due to PRs #4953 and #5439 is quite likely the last major pybind11 ABI break.

Is there a way to check the ABI version of a built extension module?

Sorry I don't know. I only know that the information is in a Python state dict:

• pybind11/include/pybind11/detail/internals.h

  Lines 387 to 406 in 34a118f

  	inline object get_python_state_dict() {
  	object state_dict;
  	#if PYBIND11_INTERNALS_VERSION <= 4 || defined(PYPY_VERSION) || defined(GRAALVM_PYTHON)
  	state_dict = reinterpret_borrow<object>(PyEval_GetBuiltins());
  	#else
  	# if PY_VERSION_HEX < 0x03090000
  	PyInterpreterState *istate = _PyInterpreterState_Get();
  	# else
  	PyInterpreterState *istate = PyInterpreterState_Get();
  	# endif
  	if (istate) {
  	state_dict = reinterpret_borrow<object>(PyInterpreterState_GetDict(istate));
  	}
  	#endif
  	if (!state_dict) {
  	raise_from(PyExc_SystemError, "pybind11::detail::get_python_state_dict() FAILED");
  	throw error_already_set();
  	}
  	return state_dict;
  	}

I don't know if the state dict is accessible from the Python (script) level.

The first exception happens, when a C++ object (module A) is passed to a method of a trampoline class (deriving from a class exposed in module B).

Assuming you want the trampoline bug fixes on the smart_holder branch: both extensions need to use the smart_holder branch.

Can you point me to corresponding unit tests of the conduit feature?

https://github.com/pybind/pybind11/pull/5296/files

(I think the test filenames haven't change in the follow-on PRs.)

I will try to come up with a minimal unit test for these during the coming week.

Not sure that's worth an effort? — I'm trying again to get the smart_holder branch merged.

[rwgk]

I don't know if the state dict is accessible from the Python (script) level.

ChatGPT doesn't think so:

https://chatgpt.com/share/67b37c8f-31a4-8008-adb1-d6d99d444597

There is no direct public Python API at the script level to access PyInterpreterState or its state dictionary, as it is considered an internal CPython structure.

[rwgk]

I guess we could unconditionally add a magic _pybind11_abi_info_() function to all extension modules that use the PYBIND11_MODULE macro, exposing PYBIND11_PLATFORM_ABI_ID, PYBIND11_INTERNALS_VERSION, and PYBIND11_INTERNALS_ID. That'd be a pretty simple PR (against master), probably.

[rwgk]

I looked back on the smart_holder branch to remind myself, keeping track of it here for easy future reference:

• When the smart_holder branch was first established (PR Adding py::smart_holder (for smart-pointer interoperability). #2672) there wasn't anything special for PYBIND11_INTERNALS_VERSION.

• PR Connecting PYBIND11_INTERNALS_VERSION to PYBIND11_USE_SMART_HOLDER_AS_DEFAULT. #2939 introduced PYBIND11_INTERNALS_VERSION 1004 iff smart_holder is the default holder.

• PR [smart_holder] Introducing PYBIND11_INTERNALS_SH_DEF. #3283 changed that by introducing PYBIND11_INTERNALS_SH_DEF. This was to accommodate PR Eliminate duplicate TLS keys for loader_life_support stack #3275, which made it possible to use internals version 4 (old) or 5 (new). (Note for completeness: PR 3283 had a silly bug, fixed only much later with PR [smart_holder] Fix PYBIND11_INTERNALS_SH_DEF mixup. #5026).

• PR [smart_holder] Bake smart_holder functionality into class_ and type_caster_base #5257 fundamentally changed the situation: the internals version now needs to be bumped even if smart_holder is not the default holder (see the PR 5257 description for why this couldn't possibly be avoided). At the time 5 was still the highest version on master, so I picked 6 for the smart_holder branch (hoping to merge into master soon).

• Then Added exception translator specific mutex used with try_translate_exceptions #5362 changed the highest version on master to 6, but I didn't do anything on the smart_holder branch (it slipped my mind).

• When I realized while working on PR Make PYBIND11_INTERNALS_VERSION 6 the default on all platforms. #5512, I took an easy way out, with the idea to go with "highest on master + 100" for now, although ideally this would be something akin to PYBIND11_INTERNALS_VERSION on master + 100, but that's something the preprocessor does not directly support.

The only way to avoid this weird complexity is to merge smart_holder into master. But at least, since #5296 it doesn't matter much anymore if the smart_holder internals version is different. We need to be careful though to correctly reflect internals changes on master and internals changes on the smart_holder branch in the internals version numbers.

[rhaschke]

Thanks for the reasonable explanation.

[rwgk]

To close the loop here:

• PR Start pybind11v3: Remove all code for PYBIND11_INTERNALS_VERSIONs 4 and 5 #5530 was merged on master.
• Based on that, PR [smart_holder] Simplification: Enable smart_holder functionality unconditionally. #5531 was merged on the smart_holder branch.
• Then PR [smart_holder] Remove obsolete detail::type_info::default_holder member. #5541 was merged on the smart_holder branch, which changed the PYBIND11_INTERNALS_VERSION to 7.

(These are steps towards merging smart_holder into master.)

[rhaschke]

I only found time today to re-evaluate different combinations of the master and smart_holder branch:

master	vInt	smart_holder	vInt	result
2.13.6	6!	ad0d55a	106	failing
c316cf3 #5512	6	ad0d55a post #5512	106	failing
c316cf3 #5512	6	ad0d55a post #5512	6!	classh undefined
c316cf3 #5512	6	664876e pre #5512	6!	passing
d8565ac #5537	6	5ab036b #5531	106	failing
d8565ac #5537	6	5ab036b #5531	6!	106 is the minimum
d8565ac #5537	106!	5ab036b #5531	106	passing
d8565ac #5537	106!	aed215c #5541	7	failing
d8565ac #5537	7!	aed215c #5541	7	passing

It turns out that they are only compatible if the PYBIND11_INTERNALS_VERSION matches. An exclamation mark indicates that the version was enforced to the specified value. This result is in contrast to your statement that the version is not important anymore for ABI compatibility:

We do have the "conduit feature" on both the master and the smart_holder branches, which means that PYBIND11_INTERNALS_VERSION isn't nearly as critical anymore as it was in the past.

[rhaschke]

Note, that I'm fine with the existing functionality, i.e. requiring matching PYBIND11_INTERNALS_VERSIONs.
It just doesn't correspond to your statements, which caused the confusion for both of us.

[rwgk]

Oof, I think you're right, that's an aspect of cross-extension functionality vs conduit feature that I overlooked. Thanks a lot for the test case. I had to think about this for a a few minutes, but it's now blazingly obvious what the shortcoming is:

Primarily:

• Your test in 5544 needs a C++-to-Python conversion for A (when processing b.a).

• The conduit feature only helps with Python-to-C++ conversions. (I knew that.)

Going deeper:

• py::class_<A> only exists in the exo_planet_pybind11 extension.

• There is no py::class_<A> in the home_planet extension, therefore that module does not know by itself how to do the C++-to-Python conversion.

• Now, what I didn't consider before: If the internals versions match between home_planet and exo_planet_pybind11, the py::class_<A> in the exo_planet_pybind11 extension goes into the shared internals, and the C++-to-Python conversion also works for home_planet.

What we'd need to make your test work is a mechanism that finds alternative internals (in the Python state dict in which the internals live), to see if A is registered somewhere, with matching PYBIND11_PLATFORM_ABI_ID. That's a bigger change I'm afraid.

Previously I was thinking the conduit feature covers the vast majority of use cases.

Your test case sadly convinces me it's a significantly smaller fraction. Sorry for missing that.

[rhaschke]

It's good that we are on the same page again 😉

What we'd need to make your test work is a mechanism that finds alternative internals (in the Python state dict in which the internals live), to see if A is registered somewhere, with matching PYBIND11_PLATFORM_ABI_ID.

I wouldn't even know where to start looking at.

[rwgk]

Currently we only have a state dict entry with key PYBIND11_INTERNALS_ID, we also need one with key PYBIND11_PLATFORM_ABI_ID. The corresponding values would be Python callables that received a C++ pointer in a capsule, to wrap them in a Python object.

I expect many devils in the details, in particular:

• What if there are two extensions with the same PYBIND11_PLATFORM_ABI_ID, but different ways of wrapping the object?

---

Currently it's very unlikely that I'll get a chance to work on that.

For now, I'll make a pass through the PR descriptions that mention the conduit feature, to emphasize that it helps with Python-to-C++ conversions only.

[rwgk]

I just changed the title of #5544.

The conduit feature is fine as is, I just failed to explain accurately what it covers.

We need an additional feature to make the #5544 test work.