From ea24ee2acca84686d90e66cbdc1a16ef80b62409 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Fri, 29 Jan 2021 02:01:48 +0100 Subject: [PATCH 1/5] Demonstrate issue with weakref constructor overloads --- tests/test_pytypes.cpp | 10 ++++++++++ tests/test_pytypes.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index d8fd77a9bb..f07b1fba2e 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -424,4 +424,14 @@ TEST_SUBMODULE(pytypes, m) { m.def("pass_to_pybind11_bytes", [](py::bytes b) { return py::len(b); }); m.def("pass_to_pybind11_str", [](py::str s) { return py::len(s); }); m.def("pass_to_std_string", [](std::string s) { return s.size(); }); + + // test_weakref + m.def("weakref_from_handle", + [](py::handle h) { return py::weakref(h); }); + m.def("weakref_from_handle_and_function", + [](py::handle h, py::function f) { return py::weakref(h, f); }); + m.def("weakref_from_object", + [](py::object o) { return py::weakref(o); }); + m.def("weakref_from_object_and_function", + [](py::object o, py::function f) { return py::weakref(o, f); }); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 301015ae4d..2cf7fcaffa 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -541,3 +541,45 @@ def test_pass_bytes_or_unicode_to_string_types(): else: with pytest.raises(TypeError): m.pass_to_pybind11_str(malformed_utf8) + + +def test_weakref(): + from weakref import getweakrefcount + + # Apparently, you cannot weakly reference an object() + class WeaklyReferenced(object): + pass + + called = [0] + + def callback(wr): + # No `nonlocal` in Python 2 + called[0] += 1 + + obj = WeaklyReferenced() + assert getweakrefcount(obj) == 0 + wr = m.weakref_from_handle(obj) # noqa: F841 + assert getweakrefcount(obj) == 1 + + obj = WeaklyReferenced() + assert getweakrefcount(obj) == 0 + wr = m.weakref_from_handle_and_function(obj, callback) # noqa: F841 + assert getweakrefcount(obj) == 1 + assert called[0] == 0 + del obj + pytest.gc_collect() + assert called[0] == 1 + + obj = WeaklyReferenced() + assert getweakrefcount(obj) == 0 + wr = m.weakref_from_object(obj) # noqa: F841 + assert getweakrefcount(obj) == 1 + + obj = WeaklyReferenced() + assert getweakrefcount(obj) == 0 + wr = m.weakref_from_object_and_function(obj, callback) # noqa: F841 + assert getweakrefcount(obj) == 1 + assert called[0] == 1 + del obj + pytest.gc_collect() + assert called[0] == 2 From b7fd776a4d22ee65ab2826505d64e68f27692159 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Fri, 29 Jan 2021 02:36:47 +0100 Subject: [PATCH 2/5] Fix weakref constructor to convert on being passed a non-weakref object --- include/pybind11/pytypes.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index fce3fa2d34..37274e3fe6 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1168,11 +1168,16 @@ class float_ : public object { class weakref : public object { public: - PYBIND11_OBJECT_DEFAULT(weakref, object, PyWeakref_Check) + PYBIND11_OBJECT_CVT(weakref, object, PyWeakref_Check, raw_weakref) explicit weakref(handle obj, handle callback = {}) : object(PyWeakref_NewRef(obj.ptr(), callback.ptr()), stolen_t{}) { if (!m_ptr) pybind11_fail("Could not allocate weak reference!"); } + +private: + static PyObject *raw_weakref(PyObject *o) { + return PyWeakref_NewRef(o, nullptr); + } }; class slice : public object { From 8827a95cfe0bdb3043e83ec241f60000f838b67c Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Fri, 29 Jan 2021 16:03:31 +0100 Subject: [PATCH 3/5] Improve on nonlocal-scoped variable in test_weakref --- tests/test_pytypes.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 2cf7fcaffa..ed51fd7360 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -550,11 +550,11 @@ def test_weakref(): class WeaklyReferenced(object): pass - called = [0] - def callback(wr): # No `nonlocal` in Python 2 - called[0] += 1 + callback.called += 1 + + callback.called = 0 obj = WeaklyReferenced() assert getweakrefcount(obj) == 0 @@ -565,10 +565,10 @@ def callback(wr): assert getweakrefcount(obj) == 0 wr = m.weakref_from_handle_and_function(obj, callback) # noqa: F841 assert getweakrefcount(obj) == 1 - assert called[0] == 0 + assert callback.called == 0 del obj pytest.gc_collect() - assert called[0] == 1 + assert callback.called == 1 obj = WeaklyReferenced() assert getweakrefcount(obj) == 0 @@ -579,7 +579,7 @@ def callback(wr): assert getweakrefcount(obj) == 0 wr = m.weakref_from_object_and_function(obj, callback) # noqa: F841 assert getweakrefcount(obj) == 1 - assert called[0] == 1 + assert callback.called == 1 del obj pytest.gc_collect() - assert called[0] == 2 + assert callback.called == 2 From cbf442ac5962c77d7cbf6f39ba5526c6553a6a56 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Sun, 31 Jan 2021 22:07:27 +0100 Subject: [PATCH 4/5] Keep backwards-compatibility by introducing PYBIND11_OBJECT_CVT_DEFAULT macro --- include/pybind11/pytypes.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 37274e3fe6..23a09bd8b8 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -819,6 +819,10 @@ PYBIND11_NAMESPACE_END(detail) : Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \ { if (!m_ptr) throw error_already_set(); } +#define PYBIND11_OBJECT_CVT_DEFAULT(Name, Parent, CheckFun, ConvertFun) \ + PYBIND11_OBJECT_CVT(Name, Parent, CheckFun, ConvertFun) \ + Name() : Parent() { } + #define PYBIND11_OBJECT_CHECK_FAILED(Name, o_ptr) \ ::pybind11::type_error("Object of type '" + \ ::pybind11::detail::get_fully_qualified_tp_name(Py_TYPE(o_ptr)) + \ @@ -1168,7 +1172,7 @@ class float_ : public object { class weakref : public object { public: - PYBIND11_OBJECT_CVT(weakref, object, PyWeakref_Check, raw_weakref) + PYBIND11_OBJECT_CVT_DEFAULT(weakref, object, PyWeakref_Check, raw_weakref) explicit weakref(handle obj, handle callback = {}) : object(PyWeakref_NewRef(obj.ptr(), callback.ptr()), stolen_t{}) { if (!m_ptr) pybind11_fail("Could not allocate weak reference!"); From e78259980dfa1d9f03cbf0382fd8e0282d55c54c Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Sun, 31 Jan 2021 22:21:11 +0100 Subject: [PATCH 5/5] Simplify test_weakref --- tests/test_pytypes.py | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index ed51fd7360..cebbd4791e 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -543,7 +543,14 @@ def test_pass_bytes_or_unicode_to_string_types(): m.pass_to_pybind11_str(malformed_utf8) -def test_weakref(): +@pytest.mark.parametrize( + "create_weakref, create_weakref_with_callback", + [ + (m.weakref_from_handle, m.weakref_from_handle_and_function), + (m.weakref_from_object, m.weakref_from_object_and_function), + ], +) +def test_weakref(create_weakref, create_weakref_with_callback): from weakref import getweakrefcount # Apparently, you cannot weakly reference an object() @@ -552,34 +559,19 @@ class WeaklyReferenced(object): def callback(wr): # No `nonlocal` in Python 2 - callback.called += 1 - - callback.called = 0 - - obj = WeaklyReferenced() - assert getweakrefcount(obj) == 0 - wr = m.weakref_from_handle(obj) # noqa: F841 - assert getweakrefcount(obj) == 1 - - obj = WeaklyReferenced() - assert getweakrefcount(obj) == 0 - wr = m.weakref_from_handle_and_function(obj, callback) # noqa: F841 - assert getweakrefcount(obj) == 1 - assert callback.called == 0 - del obj - pytest.gc_collect() - assert callback.called == 1 + callback.called = True obj = WeaklyReferenced() assert getweakrefcount(obj) == 0 - wr = m.weakref_from_object(obj) # noqa: F841 + wr = create_weakref(obj) # noqa: F841 assert getweakrefcount(obj) == 1 obj = WeaklyReferenced() assert getweakrefcount(obj) == 0 - wr = m.weakref_from_object_and_function(obj, callback) # noqa: F841 + callback.called = False + wr = create_weakref_with_callback(obj, callback) # noqa: F841 assert getweakrefcount(obj) == 1 - assert callback.called == 1 + assert not callback.called del obj pytest.gc_collect() - assert callback.called == 2 + assert callback.called