Fix exception handling when pybind11::weakref() fails. (#3739)

* Clear Python error state if pybind11::weakref() fails.

The weakref() constructor calls pybind11_fail() without clearing any
Python interpreter error state. If a client catches the C++ exception
thrown by pybind11_fail(), the Python interpreter will be left in an
error state.

* Add test case for failing to create weakref

* Add Debug asserts for pybind11 fail

* Make error handling more pythonic

* Does this fix PyPy?

* Adapt test to PyPy differences

* Simplify test to remove redundancy

Co-authored-by: Aaron Gokaslan <[email protected]>

Author: hawkinsp

include/pybind11/detail/common.h

@@ -936,9 +936,11 @@ PYBIND11_RUNTIME_EXCEPTION(cast_error, PyExc_RuntimeError) /// Thrown when pybin
 PYBIND11_RUNTIME_EXCEPTION(reference_cast_error, PyExc_RuntimeError) /// Used internally
 
 [[noreturn]] PYBIND11_NOINLINE void pybind11_fail(const char *reason) {
+    assert(!PyErr_Occurred());
     throw std::runtime_error(reason);
 }
 [[noreturn]] PYBIND11_NOINLINE void pybind11_fail(const std::string &reason) {
+    assert(!PyErr_Occurred());
     throw std::runtime_error(reason);
 }
 

include/pybind11/pytypes.h

@@ -1509,6 +1509,9 @@ class weakref : public object {
     explicit weakref(handle obj, handle callback = {})
         : object(PyWeakref_NewRef(obj.ptr(), callback.ptr()), stolen_t{}) {
         if (!m_ptr) {
+            if (PyErr_Occurred()) {
+                throw error_already_set();
+            }
             pybind11_fail("Could not allocate weak reference!");
         }
     }

tests/test_pytypes.py

@@ -1,8 +1,9 @@
+import contextlib
 import sys
 
 import pytest
 
-import env  # noqa: F401
+import env
 from pybind11_tests import debug_enabled
 from pybind11_tests import pytypes as m
 
@@ -583,6 +584,31 @@ def callback(wr):
     assert callback_called
 
 
+@pytest.mark.parametrize(
+    "create_weakref, has_callback",
+    [
+        (m.weakref_from_handle, False),
+        (m.weakref_from_object, False),
+        (m.weakref_from_handle_and_function, True),
+        (m.weakref_from_object_and_function, True),
+    ],
+)
+def test_weakref_err(create_weakref, has_callback):
+    class C:
+        __slots__ = []
+
+    def callback(_):
+        pass
+
+    ob = C()
+    # Should raise TypeError on CPython
+    with pytest.raises(TypeError) if not env.PYPY else contextlib.nullcontext():
+        if has_callback:
+            _ = create_weakref(ob, callback)
+        else:
+            _ = create_weakref(ob)
+
+
 def test_cpp_iterators():
     assert m.tuple_iterator() == 12
     assert m.dict_iterator() == 305 + 711