Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 17 additions & 0 deletions tests/test_call_policies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

#include "pybind11_tests.h"

#include <string>

struct CustomGuard {
static bool enabled;

Expand Down Expand Up @@ -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<py::gil_scoped_release>());
#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_<KeepAliveOverload>(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>());
}
13 changes: 13 additions & 0 deletions tests/test_call_policies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading