Skip to content

fix: use the internals registry in type_is_managed_by_our_internals() - #6229

Merged
henryiii merged 2 commits into
pybind:masterfrom
henryiii:fix/conduit-registry-lookup
Oct 31, 2026
Merged

henryiii merged 2 commits into
pybind:masterfrom
henryiii:fix/conduit-registry-lookup

Conversation

@henryiii

@henryiii henryiii commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

— codex GPT-6 on behalf of Ralf —

Description

On non-PyPy builds, type_is_managed_by_our_internals() compared tp_new with this module's pybind11_object_new. Each extension module has its own copy of that function. A type whose pybind11_object base was created by a different module (which shares the same internals) inherits the other module's pointer, so the check gave the wrong result in all modules except the one that created the internals. The conduit lookup then used the generic getattr path for types that pybind11 manages.

This change uses the registered_types_py lookup, which was already the PyPy path, on all platforms. It also removes the forward declaration of pybind11_object_new from cpp_conduit.h because nothing uses it now.

The existing PyPy registry path had a second bug: registered_types_py also caches empty vectors for types with no registered pybind11 bases. An empty entry was incorrectly treated as evidence that the type was managed, which could cause its Python-defined conduit method to be ignored. The lookup now also requires !it->second.empty().

The cross-module test subclasses Traveler in Python and overrides _pybind11_conduit_v1_ with a plain function. Then it calls get_points from pybind11_tests and from home_planet_very_lonely_traveler. Both modules must skip the override. Without the fix, home_planet_very_lonely_traveler calls it.

An additional regression uses a Python proxy that forwards the conduit method to a wrapped Traveler. Conversion must succeed both before and after caching that the proxy type has no registered pybind11 bases. It fails after caching if the nonempty-entry check is removed.

From finding B5 in #6104.

Suggested changelog entry:

  • Fixed type_is_managed_by_our_internals() for modules that share internals but did not create the pybind11_object base type. It now uses the internals registry on all platforms.
  • Fixed conduit lookup for types with an empty cached type-info entry on PyPy; such types must remain eligible for Python-defined conduit methods.

henryiii and others added 2 commits October 30, 2026 21:51
Comparing tp_new against this module's pybind11_object_new fails for types
whose pybind11_object base was created by another module (the inherited
pointer belongs to that module's copy). The registry lookup, which was the
PyPy path, is correct across modules and does not read type-object fields.

See pybind#6104 (B5).

Assisted-by: ClaudeCode:claude-opus-5-5
— codex GPT-6 on behalf of Ralf —

Check that a Python proxy can still forward conduit conversions after
all_type_info caches that its type has no registered pybind11 bases.
The same conversion succeeds before and after populating the empty entry.

The regression fails if the nonempty-entry condition is removed.
@rwgk

rwgk commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator

— codex GPT-6 on behalf of Ralf —

Added 3c34d7f2 to cover the second fix: an empty cached registered_types_py entry must not classify a type as managed and suppress its Python-defined conduit method. The test checks successful conversion through a Python proxy before and after populating that empty entry.

I rebuilt and ran the full local default and free-threaded CPython 3.14.4 suites (GCC 13.3, C++20), temporarily removing only && !it->second.empty() while leaving the new test in place:

Guard removed Passed Skipped Failed
Default 1,457 6 1
Free-threaded 1,440 23 1

In both runs, the only failure was test_conduit_with_empty_type_info_cache. Each run collected the same 1,464 cases from all 76 top-level Python test files. Comparing every case against the restored-guard run showed that all older test outcomes, including skips, were unchanged. All 39 Catch2 cases passed in every run.

After restoring the guard and rebuilding, both full suites passed again: 1,458 default and 1,441 free-threaded tests. All formatting/lint checks also passed.

I updated the PR description and suggested changelog to document both fixes.

@rwgk rwgk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for catching the two bugs!

@henryiii
henryiii merged commit faf0ae6 into pybind:master Oct 31, 2026
87 checks passed
@henryiii
henryiii deleted the fix/conduit-registry-lookup branch October 31, 2026 20:14
@github-actions github-actions Bot added the needs changelog Possibly needs a changelog entry label Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs changelog Possibly needs a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants