Skip to content

Commit 0184c02

Browse files
committed
Harden PYBIND11_MODULE_PYINIT and get_internals() against crashes during module init (pybind#6018)
* Wrap ensure_internals() in try-catch in PYBIND11_MODULE_PYINIT Previously, ensure_internals() was called without exception handling in the PyInit_* function (PYBIND11_MODULE_PYINIT), while the same call in PYBIND11_MODULE_EXEC was already wrapped in try-catch. On MSVC, a C++ exception propagating through the extern "C" PyInit_* boundary is undefined behavior, which can manifest as an access violation instead of a clean error message. This is a potential contributor to crashes like pybindgh-5993. Wrap the entire PyInit body in try/catch using the existing PYBIND11_CATCH_INIT_EXCEPTIONS pattern. Made-with: Cursor * Add nullptr guards in get_internals() for better crash diagnostics Add explicit null checks after get_pp() and create_pp_content_once() in get_internals(), calling pybind11_fail() with descriptive messages. These guards convert potential null-pointer dereferences (which produce unhelpful access-violation crashes, especially on Windows) into clear runtime_error messages that can be caught and reported as ImportError by the try-catch added in the previous commit. Made-with: Cursor
1 parent ec875b6 commit 0184c02

2 files changed

Lines changed: 26 additions & 14 deletions

File tree

include/pybind11/detail/common.h

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -449,19 +449,24 @@ PyModuleDef_Init should be treated like any other PyObject (so not shared across
449449
static int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject *); \
450450
PYBIND11_PLUGIN_IMPL(name) { \
451451
PYBIND11_CHECK_PYTHON_VERSION \
452-
pybind11::detail::ensure_internals(); \
453-
static ::pybind11::detail::slots_array mod_def_slots = ::pybind11::detail::init_slots( \
454-
&PYBIND11_CONCAT(pybind11_exec_, name), ##__VA_ARGS__); \
455-
static PyModuleDef def{/* m_base */ PyModuleDef_HEAD_INIT, \
456-
/* m_name */ PYBIND11_TOSTRING(name), \
457-
/* m_doc */ nullptr, \
458-
/* m_size */ 0, \
459-
/* m_methods */ nullptr, \
460-
/* m_slots */ mod_def_slots.data(), \
461-
/* m_traverse */ nullptr, \
462-
/* m_clear */ nullptr, \
463-
/* m_free */ nullptr}; \
464-
return PyModuleDef_Init(&def); \
452+
try { \
453+
pybind11::detail::ensure_internals(); \
454+
static ::pybind11::detail::slots_array mod_def_slots \
455+
= ::pybind11::detail::init_slots(&PYBIND11_CONCAT(pybind11_exec_, name), \
456+
##__VA_ARGS__); \
457+
static PyModuleDef def{/* m_base */ PyModuleDef_HEAD_INIT, \
458+
/* m_name */ PYBIND11_TOSTRING(name), \
459+
/* m_doc */ nullptr, \
460+
/* m_size */ 0, \
461+
/* m_methods */ nullptr, \
462+
/* m_slots */ mod_def_slots.data(), \
463+
/* m_traverse */ nullptr, \
464+
/* m_clear */ nullptr, \
465+
/* m_free */ nullptr}; \
466+
return PyModuleDef_Init(&def); \
467+
} \
468+
PYBIND11_CATCH_INIT_EXCEPTIONS \
469+
return nullptr; \
465470
}
466471

467472
#define PYBIND11_MODULE_EXEC(name, variable) \

include/pybind11/detail/internals.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -864,14 +864,21 @@ inline internals_pp_manager<internals> &get_internals_pp_manager() {
864864
/// Return a reference to the current `internals` data
865865
PYBIND11_NOINLINE internals &get_internals() {
866866
auto &ppmgr = get_internals_pp_manager();
867-
auto &internals_ptr = *ppmgr.get_pp();
867+
auto *pp = ppmgr.get_pp();
868+
if (!pp) {
869+
pybind11_fail("get_internals: get_pp() returned nullptr");
870+
}
871+
auto &internals_ptr = *pp;
868872
if (!internals_ptr) {
869873
// Slow path, something needs fetched from the state dict or created
870874
gil_scoped_acquire_simple gil;
871875
error_scope err_scope;
872876

873877
ppmgr.create_pp_content_once(&internals_ptr);
874878

879+
if (!internals_ptr) {
880+
pybind11_fail("get_internals: create_pp_content_once() produced nullptr");
881+
}
875882
if (!internals_ptr->instance_base) {
876883
// This calls get_internals, so cannot be called from within the internals constructor
877884
// called above because internals_ptr must be set before get_internals is called again

0 commit comments

Comments
 (0)