diff --git a/python/pybind11_v2/pybind11/functional.h b/python/pybind11_v2/pybind11/functional.h index 4b3610117..64e6a7acf 100644 --- a/python/pybind11_v2/pybind11/functional.h +++ b/python/pybind11_v2/pybind11/functional.h @@ -31,12 +31,18 @@ struct func_handle { } func_handle(const func_handle &f_) { operator=(f_); } func_handle &operator=(const func_handle &f_) { - gil_scoped_acquire acq; + // Called on the way into a Regina function that expects + // a std::function argument. In such situations the + // interpreter should already be holding the GIL. + // gil_scoped_acquire acq; f = f_.f; return *this; } ~func_handle() { - gil_scoped_acquire acq; + // Called on the way into and also out of a Regina function + // that expects a std::function argument. In such situations + // the interpreter should already be holding the GIL. + // gil_scoped_acquire acq; function kill_f(std::move(f)); } }; @@ -51,7 +57,10 @@ template struct func_wrapper : func_wrapper_base { using func_wrapper_base::func_wrapper_base; Return operator()(Args... args) const { - gil_scoped_acquire acq; + // Called when a std::function is executed as a callback within + // one of Regina's own functions. In such situations Regina's + // Python bindings are responsible for ensuring the GIL is held. + // gil_scoped_acquire acq; // casts the returned object as a rvalue to the return type return hfunc.f(std::forward(args)...).template cast(); } diff --git a/python/pybind11_v2/pybind11/gil.h b/python/pybind11_v2/pybind11/gil.h index 6b0edaee4..d5bb7d7b0 100644 --- a/python/pybind11_v2/pybind11/gil.h +++ b/python/pybind11_v2/pybind11/gil.h @@ -216,4 +216,27 @@ public: #endif // PYBIND11_SIMPLE_GIL_MANAGEMENT +/** + * A less lightweight version of gil_scoped_acquire that actually checks + * whether we are already holding the GIL before trying to acquire it. + * + * This is needed in scenarios with multiple subinterpreters, where pybind11 + * could otherwise causes deadlocks because its TLS mechanism implicitly (and + * incorrectly) assumes that different subinterpreters must be running + * from different native OS threads. + * + * - Ben Burton, 30/09/2022. + */ +class safe_gil_scoped_acquire { + std::optional gil; +public: + safe_gil_scoped_acquire() { + if (! PyGILState_Check()) + gil.emplace(); + } + safe_gil_scoped_acquire(const safe_gil_scoped_acquire&) = delete; + safe_gil_scoped_acquire& operator = (const safe_gil_scoped_acquire&) = + delete; +}; + PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/python/pybind11_v2/pybind11/iostream.h b/python/pybind11_v2/pybind11/iostream.h index 1878089e3..60586b9d7 100644 --- a/python/pybind11_v2/pybind11/iostream.h +++ b/python/pybind11_v2/pybind11/iostream.h @@ -93,7 +93,7 @@ private: // This function must be non-virtual to be called in a destructor. int _sync() { if (pbase() != pptr()) { // If buffer is not empty - gil_scoped_acquire tmp; + safe_gil_scoped_acquire tmp; // This subtraction cannot be negative, so dropping the sign. auto size = static_cast(pptr() - pbase()); size_t remainder = utf8_remainder(); diff --git a/python/pybind11_v2/pybind11/pybind11.h b/python/pybind11_v2/pybind11/pybind11.h index 5219c0ff8..101de3b20 100644 --- a/python/pybind11_v2/pybind11/pybind11.h +++ b/python/pybind11_v2/pybind11/pybind11.h @@ -26,6 +26,10 @@ #include #include +// This allows us to verify that we are #including our own patched pybind11, +// not some other pybind11 that has been installed system-wide. +#define REGINA_PYBIND11 1 + #if defined(__cpp_lib_launder) && !(defined(_MSC_VER) && (_MSC_VER < 1914)) # define PYBIND11_STD_LAUNDER std::launder # define PYBIND11_HAS_STD_LAUNDER 1 @@ -37,6 +41,10 @@ # include #endif +namespace regina { + const char* pythonTypename(const std::type_info*); +} + PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) /* https://stackoverflow.com/questions/46798456/handling-gccs-noexcept-type-warning @@ -363,6 +371,16 @@ protected: std::vector strings; }; + /* Regina-specific tweak to generation of function signatures */ + inline std::string fix_regina(const std::string& modulename, const std::string& qualname) { + if (modulename == "regina.engine") + return "regina." + qualname; + else if (modulename == "regina.engine.internal") + return "regina.internal." + qualname; + else + return modulename + "." + qualname; + } + /// Register a function call with Python (generic non-templated code goes here) void initialize_generic(unique_function_record &&unique_rec, const char *text, @@ -464,15 +482,33 @@ protected: if (!t) { pybind11_fail("Internal error while parsing type signature (1)"); } - if (auto *tinfo = detail::get_type_info(*t)) { + if (const char* name = regina::pythonTypename(t)) { + signature += name; + } else if (auto *tinfo = detail::get_type_info(*t)) { handle th((PyObject *) tinfo->type); - signature += th.attr("__module__").cast() + "." - + th.attr("__qualname__").cast(); + std::string qualname; + try { + qualname = th.attr("__qualname__").cast(); + } catch (...) { + // In Python 3.12 we get an error in our stripped-down + // interpreter since PythonOutputStream is missing both + // __qualname__ and __name__. Do not make this fatal. + qualname = ""; + } + const auto m = th.attr("__module__").cast(); + signature += fix_regina(m, qualname); } else if (rec->is_new_style_constructor && arg_index == 0) { // A new-style `__init__` takes `self` as `value_and_holder`. // Rewrite it to the proper class type. - signature += rec->scope.attr("__module__").cast() + "." - + rec->scope.attr("__qualname__").cast(); + std::string qualname; + try { + qualname = rec->scope.attr("__qualname__").cast(); + } catch (...) { + // See above for the explanation here. + qualname = ""; + } + const auto m = rec->scope.attr("__module__").cast(); + signature += fix_regina(m, qualname); } else { signature += detail::quote_cpp_type_name(detail::clean_type_id(t->name())); } @@ -1021,6 +1057,16 @@ protected: } catch (abi::__forced_unwind &) { throw; #endif + } catch (const pybind11::stop_iteration& stop) { + /* We prioritise catching stop_iteration before any of the other + exception logic below, since stop_iteration is arguably the one + setting where exceptions are normal as opposed to "exceptional". + The default exception logic involves many try/catch/rethrow + blocks, and in settings where try/catch is slow (e.g., running + within SageMath on macOS), this can add to a noticeable + performance penalty when using iterators. */ + PyErr_SetString(PyExc_StopIteration, stop.what()); + return nullptr; } catch (...) { try_translate_exceptions(); return nullptr; @@ -2726,13 +2772,13 @@ void print(Args &&...args) { inline void error_already_set::m_fetched_error_deleter(detail::error_fetch_and_normalize *raw_ptr) { - gil_scoped_acquire gil; + safe_gil_scoped_acquire gil; error_scope scope; delete raw_ptr; } inline const char *error_already_set::what() const noexcept { - gil_scoped_acquire gil; + safe_gil_scoped_acquire gil; error_scope scope; return m_fetched_error->error_string().c_str(); } @@ -2862,7 +2908,7 @@ function get_override(const T *this_ptr, const char *name) { #define PYBIND11_OVERRIDE_IMPL(ret_type, cname, name, ...) \ do { \ - pybind11::gil_scoped_acquire gil; \ + pybind11::safe_gil_scoped_acquire gil; \ pybind11::function override \ = pybind11::get_override(static_cast(this), name); \ if (override) { \