fix: never run keep_alive for a failed overload or failed return conversion by henryiii · Pull Request #6183 · pybind/pybind11

henryiii · 2026-09-23T17:34:58Z

🤖 AI text below 🤖

Description

This PR contains the commit from, and closes #6154 by @jturney. It adds more commits on top.

#6154: a py::keep_alive with index 0 on an overloaded function causes a segfault if an earlier overload fails argument conversion. The dispatcher calls postcall with the PYBIND11_TRY_NEXT_OVERLOAD sentinel ((PyObject *) 1), and keep_alive_impl dereferences it.

The added commits extend that fix:

The dispatcher now runs postcall only for the overload that matched. Before, it also ran postcall with the sentinel for an overload that failed argument conversion.
The precall hook now runs in call_impl after load_args succeeds, just before the call. Before, an argument-to-argument keep_alive (for example keep_alive<1, 2>) ran before argument conversion, so it could set up a reference for an overload that then failed. It still runs before the call, so a keep_alive error stops the call before it has side effects.
keep_alive_impl skips a null return handle. A failed return-value conversion now raises its own error, not "Could not activate keep_alive!".
If a keep_alive with index 0 fails after the call, the return value is released. Before, it leaked.
The precall and postcall documentation now says when each runs, and that the postcall handle is null if return-value conversion failed.

A custom process_attribute now runs its precall after argument conversion, not before it, and its postcall only for the overload that matched. keep_alive is the only built-in attribute that uses precall.

Tests cover the return value as nurse and as patient, a rejected sole candidate, the argument-to-argument case, the failed return conversion, and a failing keep_alive (no side effects, no leaked return value).

Closes #6154

Suggested changelog entry:

Fixed a segfault when a function with py::keep_alive fails argument conversion. py::keep_alive now runs only for the overload that matched, and a failing py::keep_alive no longer leaks the return value. A custom process_attribute now runs its precall after argument conversion, and its postcall only for the overload that matched.

📚 Documentation preview 📚: https://pybind11--6183.org.readthedocs.build/

…rsion Calling an overloaded function bound with `py::keep_alive<0, N>` segfaults whenever the call is not matched by the first overload tried. .def("make", &Holder::from_int, py::keep_alive<0, 1>()) .def("make", &Holder::from_string, py::keep_alive<0, 1>()); h.make(1) # fine h.make("x") # SIGSEGV The dispatch lambda in `cpp_function::initialize` invokes the post-call hook unconditionally: auto result = call_impl<...>(call, ...); process_attributes<Extra...>::postcall(call, result); but `call_impl` returns `PYBIND11_TRY_NEXT_OVERLOAD` when `load_args` fails, and that sentinel is `((PyObject *) 1)` rather than an object. For a `keep_alive` whose nurse or patient is index 0 the work happens in postcall, so `keep_alive_impl` receives the sentinel as `ret`, hands it to `get_arg(0)` and dereferences it in `_Py_TYPE`. The existing guards do not catch it: the sentinel is neither null nor `Py_None`, so it passes straight through the checks added in pybind#341. Guard inside `keep_alive_impl`, next to those checks. Only the `Nurse == 0 || Patient == 0` specialization does its work in postcall, and `keep_alive` is the only call policy with a non-trivial postcall, so this covers every path that can observe the sentinel. `keep_alive<1, 2>` and friends run in precall against fully populated `call.args` and are unaffected. The regression test crashes the interpreter without the fix. Reaching the second overload is what matters: it is the first overload's failed conversion that produces the sentinel.

…ersion Move keep_alive activation entirely to postcall, so an argument-to-argument keep_alive no longer fires in precall for an overload that later fails argument conversion. Extend the postcall guard to also skip a null return handle, so a failed return-value conversion raises its own error instead of "Could not activate keep_alive!". Document that postcall can receive the PYBIND11_TRY_NEXT_OVERLOAD sentinel or a null handle. Tests cover the sentinel as nurse and as patient, the argument-to-argument case, and the failed return conversion, and no longer make the test object its own patient. Assisted-by: ClaudeCode:claude-fable-5 Claude-Session: https://claude.ai/code/session_01WRPhwYhrJKvFXU2NWe1p3W

Moving keep_alive<N, P> (N, P != 0) to postcall meant a keep_alive error was raised after the function ran, so its side effects stayed. Run the precall hook in call_impl after load_args succeeds instead, so a failed overload still does not trigger it and an error still stops the call. Also release the return value when a postcall keep_alive throws, as the dispatcher drops it. Assisted-by: ClaudeCode:claude-opus-5-5

Assisted-by: ClaudeCode:claude-opus-5-5

itamaro

thanks for folding #6154 in! we hit the overload-fallback segfault on 3.1.0 and have been carrying a backport of 66811ce, so this is timely :)

nit: the changelog entry should probably mention that custom process_attribute::precall now runs after argument conversion.

itamaro · 2026-09-28T18:56:04Z

                                    extract_guard_t<Extra...>,
-                                    cast_in>(call, detail::function_ref<Return(Args...)>(cap->f));
+                                    cast_in>(call,
+                                             detail::function_ref<Return(Args...)>(cap->f),
+                                             &process_attributes<Extra...>::precall);

            /* Invoke call policy post-call hook */
            process_attributes<Extra...>::postcall(call, result);


now that precall runs only for the matched overload, should postcall skip PYBIND11_TRY_NEXT_OVERLOAD too?

itamaro · 2026-09-28T19:00:34Z

+    /// The handle is not always a valid object: it is the PYBIND11_TRY_NEXT_OVERLOAD sentinel
+    /// if argument conversion failed, and null if the call or return-value conversion failed.


nit: a throwing call never reaches postcall, so null only comes from a failed return conversion. Maybe "…and null if return-value conversion failed"?

itamaro · 2026-09-28T19:01:32Z

+    // With index 0, this runs in postcall, which the dispatcher runs even when the overload
+    // produced no value: `ret` is the PYBIND11_TRY_NEXT_OVERLOAD sentinel ((PyObject *) 1) if
+    // the overload bailed out of `load_args`, and null if the return-value conversion failed
+    // (with the real error already set). There is no relationship to establish then. The
+    // sentinel would pass the guards below and be dereferenced, and a null `ret` would be
+    // reported as "Could not activate keep_alive!", masking the real error.
+    const bool uses_ret = Nurse == 0 || Patient == 0;
+    if (uses_ret && (!ret || ret.ptr() == PYBIND11_TRY_NEXT_OVERLOAD)) {
+        return;
+    }


nit: maybe note that uses_ret is needed because the precall path passes handle()? Otherwise it looks simplifiable

itamaro · 2026-09-28T19:02:59Z

        assert m.without_gil() == "GIL released"
+
+
+def test_keep_alive_failed_overload():


could we cover a rejected sole candidate too? It also crashed on 3.1.0

with pytest.raises(TypeError): m.keep_alive_single(obj, "x") m.def( "keep_alive_single", [](const KeepAliveOverload &, int) { return KeepAliveOverload(); }, py::keep_alive<0, 1>());

The dispatcher now runs postcall only for the overload that matched, like precall. Add a test for a rejected sole candidate, and clarify the postcall and keep_alive_impl comments. Assisted-by: ClaudeCode:claude-opus-5-5

pybind11 3.1.0 runs keep_alive's postcall on the try-next-overload sentinel, so any binding with keep_alive<0, N> segfaults when called with arguments that match no overload -- Node.addObject(1.0) in SofaPython3. Fixed upstream by pybind/pybind11#6183. The runtime test now checks that such a call raises TypeError. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

brentDSL · 2026-10-04T06:11:14Z

Downstream confirmation from SofaPython3 (SOFA's bindings), which hits this through Node.addObject, bound with keep_alive<0, 2>.

SOFA master + SofaPython3 master built from source on CPython 3.14.7 (linux-64, conda-forge toolchain):

	pybind11 v3.1.0	this PR (`a7e62c8`)
`node.addObject(1.0)`	segfault	`TypeError`
`Bindings.Sofa.Tests`	segfault at test 107 (`test_createObjectInvalid`)	148/148 pass
`Bindings.Modules.Tests`	9/11 (2 failures from scipy missing in my env, unrelated)	11/11 pass
`Bindings.SofaRuntime.Tests`	2/2	2/2
`Bindings.SofaTypes.Tests`	6/6	6/6

A minimal keep_alive<0, 1> reproducer raises TypeError with v3.0.4 and segfaults with v3.1.0 on both 3.12 and 3.14, consistent with the regression arriving with #5887. In the meantime we are building conda-forge's sofa-python3 against pybind11 <3.1. Thanks for the thorough fix.

jturney and others added 4 commits September 23, 2026 13:34

test: collect the child before capturing output on PyPy

4a9cfbf

Assisted-by: ClaudeCode:claude-opus-5-5

itamaro approved these changes Sep 28, 2026

View reviewed changes

fix: skip postcall for an overload that failed argument conversion

a7e62c8

The dispatcher now runs postcall only for the overload that matched, like precall. Add a test for a rejected sole candidate, and clarify the postcall and keep_alive_impl comments. Assisted-by: ClaudeCode:claude-opus-5-5

itamaro approved these changes Sep 29, 2026

View reviewed changes

This was referenced Oct 4, 2026

sofa-python3 v26.06: fix empty linux/osx packages, build against pybind11 <3.1, add a runtime test conda-forge/sofa-python3-feedstock#4

Open

[Pixi] Build SofaPython3 against pybind11 < 3.1 sofa-framework/sofa#6348

Open

Navigation Menu

Repository navigation

fix: never run keep_alive for a failed overload or failed return conversion - #6183

fix: never run keep_alive for a failed overload or failed return conversion#6183
henryiii wants to merge 5 commits into
pybind:masterfrom
henryiii:fix-keep-alive-failed-overload

henryiii commented Sep 23, 2026 •

edited by github-actions Bot

Loading

Uh oh!

itamaro left a comment

Uh oh!

itamaro Sep 28, 2026

Uh oh!

itamaro Sep 28, 2026

Uh oh!

itamaro Sep 28, 2026

Uh oh!

itamaro Sep 28, 2026

Uh oh!

brentDSL commented Oct 4, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		/// The handle is not always a valid object: it is the PYBIND11_TRY_NEXT_OVERLOAD sentinel
		/// if argument conversion failed, and null if the call or return-value conversion failed.

		assert m.without_gil() == "GIL released"


		def test_keep_alive_failed_overload():

Conversation

henryiii commented Sep 23, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Suggested changelog entry:

Uh oh!

itamaro left a comment

Choose a reason for hiding this comment

Uh oh!

itamaro Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

itamaro Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

itamaro Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

itamaro Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

brentDSL commented Oct 4, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

henryiii commented Sep 23, 2026 •

edited by github-actions Bot

Loading