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

docs/advanced/functions.rst

@@ -233,6 +233,12 @@ is equivalent to the following pseudocode:
         return foo(args...); // forwarded arguments
     });
 
+Argument conversion, including extraction from type casters, finishes before
+the guards are constructed. A candidate rejected during argument conversion
+does not construct its guards. Call-policy ``precall`` hooks also run before
+guard construction, and the guards are destroyed before Python return-value
+conversion.
+
 The only requirement is that ``T`` is default-constructible, but otherwise any
 scope guard will work. This is very useful in combination with ``gil_scoped_release``.
 See :ref:`gil`.

include/pybind11/attr.h

@@ -409,7 +409,9 @@ struct process_attribute_default {
     /// Default implementation: do nothing
     static void init(const T &, function_record *) {}
     static void init(const T &, type_record *) {}
+    /// Runs after argument conversion succeeded, before the call.
     static void precall(function_call &) {}
+    /// Runs after the call succeeded. The handle is null if return-value conversion failed.
     static void postcall(function_call &, handle) {}
 };
 
@@ -650,8 +652,8 @@ struct process_attribute<call_guard<Ts...>> : process_attribute_default<call_gua
 
 /**
  * Process a keep_alive call policy -- invokes keep_alive_impl during the
- * pre-call handler if both Nurse, Patient != 0 and use the post-call handler
- * otherwise
+ * pre-call handler (after argument conversion succeeded) if both Nurse,
+ * Patient != 0 and use the post-call handler otherwise
  */
 template <size_t Nurse, size_t Patient>
 struct process_attribute<keep_alive<Nurse, Patient>>

include/pybind11/cast.h

@@ -2162,17 +2162,17 @@ class argument_loader {
 
     bool load_args(function_call &call) { return load_impl_sequence(call, indices{}); }
 
-    template <typename Return, typename Guard, typename Func>
+    template <typename Return, typename Guard, typename Func, typename Precall>
     // NOLINTNEXTLINE(readability-const-return-type)
-    enable_if_t<!std::is_void<Return>::value, Return> call(Func &&f) && {
-        return std::move(*this).template call_impl<remove_cv_t<Return>>(
-            std::forward<Func>(f), indices{}, Guard{});
+    enable_if_t<!std::is_void<Return>::value, Return> call(Func &&f, Precall &&precall) && {
+        return std::move(*this).template call_impl<remove_cv_t<Return>, Guard>(
+            std::forward<Func>(f), std::forward<Precall>(precall), indices{});
     }
 
-    template <typename Return, typename Guard, typename Func>
-    enable_if_t<std::is_void<Return>::value, void_type> call(Func &&f) && {
-        std::move(*this).template call_impl<remove_cv_t<Return>>(
-            std::forward<Func>(f), indices{}, Guard{});
+    template <typename Return, typename Guard, typename Func, typename Precall>
+    enable_if_t<std::is_void<Return>::value, void_type> call(Func &&f, Precall &&precall) && {
+        std::move(*this).template call_impl<remove_cv_t<Return>, Guard>(
+            std::forward<Func>(f), std::forward<Precall>(precall), indices{});
         return void_type();
     }
 
@@ -2201,9 +2201,12 @@ class argument_loader {
         return true;
     }
 
-    template <typename Return, typename Func, size_t... Is, typename Guard>
-    Return call_impl(Func &&f, index_sequence<Is...>, Guard &&) && {
-        return std::forward<Func>(f)(cast_op<Args>(std::move(std::get<Is>(argcasters)))...);
+    template <typename Return, typename Guard, typename Func, typename Precall, size_t... Is>
+    Return call_impl(Func &&f, Precall &&precall, index_sequence<Is...>) && {
+        // Func provides function_ref::invoke_with_guard, whose typed parameters finish conversion
+        // before precall. Direct casts and returns preserve copy elision (see issue #6142).
+        return std::forward<Func>(f).template invoke_with_guard<Guard>(
+            std::forward<Precall>(precall), cast_op<Args>(std::move(std::get<Is>(argcasters)))...);
     }
 
     std::tuple<make_caster<Args>...> argcasters;

include/pybind11/detail/function_ref.h

@@ -104,6 +104,17 @@ class function_ref<Ret(Params...)> {
         return callback(callable, std::forward<Params>(params)...);
     }
 
+    template <typename Guard, typename Precall>
+    // NOLINTNEXTLINE(performance-unnecessary-value-param)
+    Ret invoke_with_guard(Precall &&precall, Params... params) const {
+        // Keep the same parameter boundary as operator() so prvalue arguments can be elided.
+        // Argument conversion must finish before the hook, which runs before the guard.
+        std::forward<Precall>(precall)();
+        Guard guard{};
+        (void) guard;
+        return callback(callable, std::forward<Params>(params)...);
+    }
+
     explicit operator bool() const { return callback; }
 
     bool operator==(const function_ref<Ret(Params...)> &Other) const {

include/pybind11/pybind11-inl.h

@@ -309,6 +309,14 @@ PYBIND11_NOINLINE_ATTR PYBIND11_INLINE void keep_alive_impl(handle nurse, handle
 
 PYBIND11_NOINLINE_ATTR PYBIND11_INLINE void
 keep_alive_impl(size_t Nurse, size_t Patient, function_call &call, handle ret) {
+    // With index 0, this runs in postcall, where a null `ret` means the return-value conversion
+    // failed with the real error already set. Report that error, not "Could not activate
+    // keep_alive!". Without index 0, this runs in precall, where `ret` is always null.
+    const bool uses_ret = Nurse == 0 || Patient == 0;
+    if (uses_ret && !ret) {
+        return;
+    }
+
     auto get_arg = [&](size_t n) {
         if (n == 0) {
             return ret;

include/pybind11/pybind11.h

@@ -319,7 +319,9 @@ class cpp_function : public function {
     // actual function lambda so that we can get code reuse for
     // functions with the same Return, Args, and Guard.
     template <typename Return, typename Guard, typename ArgsConverter, typename... Args>
-    static handle call_impl(detail::function_call &call, detail::function_ref<Return(Args...)> f) {
+    static handle call_impl(detail::function_call &call,
+                            detail::function_ref<Return(Args...)> f,
+                            void (*precall)(detail::function_call &)) {
         using namespace detail;
         // Static assertion: function_ref must be trivially copyable to ensure safe pass-by-value.
         // Lifetime safety: The function_ref is created from cap->f which lives in the capture
@@ -335,18 +337,24 @@ class cpp_function : public function {
             return PYBIND11_TRY_NEXT_OVERLOAD;
         }
 
+        // cast_op can reject an argument after load_args succeeds. Run the hook after those
+        // conversions, but before constructing the guard, which may release the GIL.
+        auto precall_hook = [&] { precall(call); };
+
         /* Override policy for rvalues -- usually to enforce rvp::move on an rvalue */
         return_value_policy policy
             = return_value_policy_override<Return>::policy(call.func.policy);
 
         /* Perform the function call */
         handle result;
         if (call.func.is_setter) {
-            (void) std::move(args_converter).template call<Return, Guard>(f);
+            (void) std::move(args_converter).template call<Return, Guard>(f, precall_hook);
             result = none().release();
         } else {
             result = cast_out::cast(
-                std::move(args_converter).template call<Return, Guard>(f), policy, call.parent);
+                std::move(args_converter).template call<Return, Guard>(f, precall_hook),
+                policy,
+                call.parent);
         }
 
         return result;
@@ -415,9 +423,6 @@ class cpp_function : public function {
 
         /* Dispatch code which converts function arguments and performs the actual function call */
         rec->impl = [](function_call &call) -> handle {
-            /* Invoke call policy pre-call hook */
-            process_attributes<Extra...>::precall(call);
-
             /* Get a pointer to the capture object */
             const auto *data = (sizeof(capture) <= sizeof(call.func.data) ? &call.func.data
                                                                           : call.func.data[0]);
@@ -427,12 +432,18 @@ class cpp_function : public function {
                                     /* Function scope guard -- defaults to the compile-to-nothing
                                        `void_type` */
                                     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);
+            if (result.ptr() == PYBIND11_TRY_NEXT_OVERLOAD) {
+                return result;
+            }
 
-            return result;
+            // Own the result while running postcall so any throwing hook releases it.
+            auto result_guard = reinterpret_steal<object>(result);
+            process_attributes<Extra...>::postcall(call, result_guard);
+            return result_guard.release();
         };
 
         rec->nargs_pos = cast_in::args_pos >= 0