diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index f57514ae28..83fe1a3d25 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -3356,6 +3356,15 @@ PYBIND11_NOINLINE void keep_alive_impl(handle nurse, handle patient) { PYBIND11_NOINLINE void keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret) { + // The overload bailed out of `load_args` before running, so `ret` is the + // PYBIND11_TRY_NEXT_OVERLOAD sentinel ((PyObject *) 1) rather than an object. There is no + // call and therefore no relationship to establish; the dispatcher will try the next + // overload. Checked here because the sentinel is neither null nor `Py_None`, so it passes + // straight through the guards below and is dereferenced. + if (ret.ptr() == PYBIND11_TRY_NEXT_OVERLOAD) { + return; + } + auto get_arg = [&](size_t n) { if (n == 0) { return ret; diff --git a/tests/test_call_policies.cpp b/tests/test_call_policies.cpp index 9140f7e9f2..44a1c0196e 100644 --- a/tests/test_call_policies.cpp +++ b/tests/test_call_policies.cpp @@ -9,6 +9,8 @@ #include "pybind11_tests.h" +#include + struct CustomGuard { static bool enabled; @@ -110,4 +112,19 @@ TEST_SUBMODULE(call_policies, m) { m.def("with_gil", report_gil_status); m.def("without_gil", report_gil_status, py::call_guard()); #endif + + // A keep_alive whose nurse or patient is the return value runs in postcall, which + // the dispatcher invokes even when the overload bailed out of load_args. `ret` is then the + // PYBIND11_TRY_NEXT_OVERLOAD sentinel rather than an object, and dereferencing it crashed. + // Reaching the SECOND overload is what exercises the failed first attempt. + struct KeepAliveOverload {}; + py::class_(m, "KeepAliveOverload").def(py::init<>()); + m.def( + "keep_alive_overload", + [](KeepAliveOverload *self, int) { return self; }, + py::keep_alive<0, 1>()); + m.def( + "keep_alive_overload", + [](KeepAliveOverload *self, const std::string &) { return self; }, + py::keep_alive<0, 1>()); } diff --git a/tests/test_call_policies.py b/tests/test_call_policies.py index 11aab9fd9c..4d3937c1c1 100644 --- a/tests/test_call_policies.py +++ b/tests/test_call_policies.py @@ -254,3 +254,16 @@ def test_call_guard(): if hasattr(m, "with_gil"): assert m.with_gil() == "GIL held" assert m.without_gil() == "GIL released" + + +def test_keep_alive_failed_overload(): + """A keep_alive on an overload that fails argument conversion must not fire. + + The dispatcher invokes postcall unconditionally, so a keep_alive<0, N> on the + overload that bails out of load_args was handed the PYBIND11_TRY_NEXT_OVERLOAD + sentinel as its return value and dereferenced it. Calling the second overload is + what makes the first one fail first. + """ + obj = m.KeepAliveOverload() + assert m.keep_alive_overload(obj, 1) is obj + assert m.keep_alive_overload(obj, "x") is obj