Zengtudor/pybind11
1
0
Fork 0
mirror of https://github.com/pybind/pybind11.git synced 2024-11-22 05:05:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: PyCapsule_GetDestructor is allowed to return a nullptr destructor (#4221)

Browse Source 
* fix: PyCapsule_GetDestructor is allowed to return a nullptr destructor

Previously, this code would error out if the destructor happened to be
a nullptr. This is incorrect. nullptrs are allowed for capsule
destructors.

"It is legal for a capsule to have a NULL destructor. This makes a
NULL return code somewhat ambiguous; use PyCapsule_IsValid() or
PyErr_Occurred() to disambiguate."

See:

https://docs.python.org/3/c-api/capsule.html#c.PyCapsule_GetDestructor

I noticed this while working on a type caster related to #3858 DLPack
happens to allow the destructor not to be defined on a capsule, and I
encountered such a case. See:

e2bdd3bee8/include/dlpack/dlpack.h (L219)

* Add test for the fix.

* Update tests/test_pytypes.cpp

I tried this locally and it works!
I never knew that there are cases where `reinterpret_cast` does not work but `static_cast` does. Let's see if all compilers are happy with this.

Co-authored-by: Aaron Gokaslan <skylion.aaron@gmail.com>

* style: pre-commit fixes

Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <rwgkio@gmail.com>
Co-authored-by: Aaron Gokaslan <skylion.aaron@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
Daniel Galvez 2022-10-07 12:27:54 -07:00 committed by GitHub
parent 4a42156209
commit 7c6f2f80a7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 23 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
include/pybind11/pytypes.h
View File

@ -1829,18 +1829,18 @@ public:
// guard if destructor called while err indicator is set // guard if destructor called while err indicator is set
error_scope error_guard; error_scope error_guard;
auto destructor = reinterpret_cast<void (*)(void *)>(PyCapsule_GetContext(o)); auto destructor = reinterpret_cast<void (*)(void *)>(PyCapsule_GetContext(o));
if (destructor == nullptr) { if (PyErr_Occurred()) {
if (PyErr_Occurred()) { throw error_already_set();
throw error_already_set();
}
pybind11_fail("Unable to get capsule context");
} }
const char *name = get_name_in_error_scope(o); const char *name = get_name_in_error_scope(o);
void *ptr = PyCapsule_GetPointer(o, name); void *ptr = PyCapsule_GetPointer(o, name);
if (ptr == nullptr) { if (ptr == nullptr) {
throw error_already_set(); throw error_already_set();
} }
destructor(ptr);
if (destructor != nullptr) {
destructor(ptr);
}
}); });
if (!m_ptr || PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { if (!m_ptr || PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) {

6
tests/test_pytypes.cpp
View File

@ -289,6 +289,12 @@ TEST_SUBMODULE(pytypes, m) {
return capsule; return capsule;
}); });
m.def("return_capsule_with_explicit_nullptr_dtor", []() {
py::print("creating capsule with explicit nullptr dtor");
return py::capsule(reinterpret_cast<void *>(1234),
static_cast<void (*)(void *)>(nullptr)); // PR #4221
});
// test_accessors // test_accessors
m.def("accessor_api", [](const py::object &o) { m.def("accessor_api", [](const py::object &o) {
auto d = py::dict(); auto d = py::dict();

11
tests/test_pytypes.py
View File

@ -299,6 +299,17 @@ def test_capsule(capture):
""" """
) )
with capture:
a = m.return_capsule_with_explicit_nullptr_dtor()
del a
pytest.gc_collect()
assert (
capture.unordered
== """
creating capsule with explicit nullptr dtor
"""
)
def test_accessors(): def test_accessors():
class SubTestObject: class SubTestObject: