Modernize pybind11 initialization of custom exceptions

Fixes some possible undefined behavior on destruction, etc.
pikepdf · Aug 23, 2024 · 59a671f · 59a671f
1 parent 44758cf
commit 59a671f
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 16 deletions.
diff --git a/pyproject.toml b/pyproject.toml
@@ -6,7 +6,7 @@ requires = [
   "setuptools >= 61",
   "setuptools < 72; python_implementation == 'PyPy'", # https://github.com/pypa/distutils/issues/283
   "wheel >= 0.37",
-  "pybind11 >= 2.10.1",
+  "pybind11 >= 2.12.0",
 ]
 build-backend = "setuptools.build_meta"
 

diff --git a/src/core/pikepdf.cpp b/src/core/pikepdf.cpp
@@ -22,6 +22,7 @@
 #include <pybind11/stl.h>
 #include <pybind11/iostream.h>
 #include <pybind11/buffer_info.h>
+#include <pybind11/gil_safe_call_once.h>
 
 #include "qpdf_pagelist.h"
 #include "utils.h"
@@ -192,46 +193,59 @@ PYBIND11_MODULE(_core, m)
         .def("_unparse_content_stream", unparse_content_stream);
 
     // -- Exceptions --
-    static py::exception<QPDFExc> exc_main(m, "PdfError");
-    static py::exception<QPDFExc> exc_password(m, "PasswordError");
-    static py::exception<QPDFExc> exc_datadecoding(m, "DataDecodingError");
-    static py::exception<QPDFUsage> exc_usage(m, "JobUsageError");
-    static py::exception<std::logic_error> exc_foreign(m, "ForeignObjectError");
-    static py::exception<std::runtime_error> exc_destroyedobject(
-        m, "DeletedObjectError");
+    // clang-format off
+    PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object> exc_main;
+    exc_main.call_once_and_store_result(
+        [&]() { return py::exception<QPDFExc>(m, "PdfError"); });
+    PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object> exc_password;
+    exc_password.call_once_and_store_result(
+        [&]() { return py::exception<QPDFExc>(m, "PasswordError"); });
+    PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object> exc_datadecoding;
+    exc_datadecoding.call_once_and_store_result(
+        [&]() { return py::exception<QPDFExc>(m, "DataDecodingError"); });
+    PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object> exc_usage;
+    exc_usage.call_once_and_store_result(
+        [&]() { return py::exception<QPDFUsage>(m, "JobUsageError"); });
+    PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object> exc_foreign;
+    exc_foreign.call_once_and_store_result(
+        [&]() { return py::exception<std::logic_error>(m, "ForeignObjectError"); });
+    PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object> exc_destroyedobject;
+    exc_destroyedobject.call_once_and_store_result(
+        [&]() { return py::exception<std::runtime_error>(m, "DeletedObjectError"); });
+    // clang-format on
     py::register_exception_translator([](std::exception_ptr p) {
         try {
             if (p)
                 std::rethrow_exception(p);
         } catch (const QPDFExc &e) {
             if (e.getErrorCode() == qpdf_e_password) {
-                py::set_error(exc_password, e.what());
+                py::set_error(exc_password.get_stored(), e.what());
             } else {
-                py::set_error(exc_main, e.what());
+                py::set_error(exc_main.get_stored(), e.what());
             }
         } catch (const QPDFSystemError &e) {
             if (e.getErrno() != 0) {
                 TemporaryErrnoChange errno_holder(e.getErrno());
                 PyErr_SetFromErrnoWithFilename(
                     PyExc_OSError, e.getDescription().c_str());
             } else {
-                py::set_error(exc_main, e.what());
+                py::set_error(exc_main.get_stored(), e.what());
             }
         } catch (const QPDFUsage &e) {
-            py::set_error(exc_usage, e.what());
+            py::set_error(exc_usage.get_stored(), e.what());
         } catch (const std::logic_error &e) {
             auto trans = translate_qpdf_logic_error(e);
             if (trans.second == error_type_foreign)
-                py::set_error(exc_foreign, trans.first.c_str());
+                py::set_error(exc_foreign.get_stored(), trans.first.c_str());
             else if (trans.second == error_type_pdferror)
-                py::set_error(exc_main, trans.first.c_str());
+                py::set_error(exc_main.get_stored(), trans.first.c_str());
             else
                 std::rethrow_exception(p);
         } catch (const std::runtime_error &e) {
             if (is_data_decoding_error(e))
-                py::set_error(exc_datadecoding, e.what());
+                py::set_error(exc_datadecoding.get_stored(), e.what());
             else if (is_destroyed_object_error(e))
-                py::set_error(exc_destroyedobject, e.what());
+                py::set_error(exc_destroyedobject.get_stored(), e.what());
             else
                 std::rethrow_exception(p);
         }