From b42b6aa45856f1eeef101c5f3428505019685776 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 10 Oct 2022 11:57:54 -0700 Subject: [PATCH 1/3] Improve pycapsule error handling corner cases --- include/pybind11/pytypes.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 29506b7eaf..5fc0abb986 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1829,7 +1829,7 @@ class capsule : public object { // guard if destructor called while err indicator is set error_scope error_guard; auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); - if (PyErr_Occurred()) { + if (destructor == nullptr && PyErr_Occurred()) { throw error_already_set(); } const char *name = get_name_in_error_scope(o); @@ -1843,7 +1843,7 @@ class capsule : public object { } }); - if (!m_ptr || PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { + if (!m_ptr || PyCapsule_SetContext(m_ptr, reinterpret_cast(destructor)) != 0) { throw error_already_set(); } } @@ -1852,10 +1852,13 @@ class capsule : public object { m_ptr = PyCapsule_New(reinterpret_cast(destructor), nullptr, [](PyObject *o) { const char *name = get_name_in_error_scope(o); auto destructor = reinterpret_cast(PyCapsule_GetPointer(o, name)); - if (destructor == nullptr) { - throw error_already_set(); + if (destructor != nullptr) { + destructor(); + } else { + if (PyErr_Occurred()) { + throw error_already_set(); + } } - destructor(); }); if (!m_ptr) { From 1a12aead906ddd640a4aacabf53a651da675cd13 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 10 Oct 2022 12:26:14 -0700 Subject: [PATCH 2/3] Handle another corner case --- include/pybind11/pytypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 5fc0abb986..eb9665e94d 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1834,7 +1834,7 @@ class capsule : public object { } const char *name = get_name_in_error_scope(o); void *ptr = PyCapsule_GetPointer(o, name); - if (ptr == nullptr) { + if (ptr == nullptr && PyErr_Occurred()) { throw error_already_set(); } From 94d47a15abf788bc5bff3fabe59e36a8d7d0dd2e Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 10 Oct 2022 12:38:58 -0700 Subject: [PATCH 3/3] Simplify err handling code --- include/pybind11/pytypes.h | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index eb9665e94d..2e6b755caf 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1834,7 +1834,7 @@ class capsule : public object { } const char *name = get_name_in_error_scope(o); void *ptr = PyCapsule_GetPointer(o, name); - if (ptr == nullptr && PyErr_Occurred()) { + if (ptr == nullptr) { throw error_already_set(); } @@ -1852,13 +1852,10 @@ class capsule : public object { m_ptr = PyCapsule_New(reinterpret_cast(destructor), nullptr, [](PyObject *o) { const char *name = get_name_in_error_scope(o); auto destructor = reinterpret_cast(PyCapsule_GetPointer(o, name)); - if (destructor != nullptr) { - destructor(); - } else { - if (PyErr_Occurred()) { - throw error_already_set(); - } + if (destructor == nullptr) { + throw error_already_set(); } + destructor(); }); if (!m_ptr) {