Skip to content

Commit e8cead1

Browse files
authored
Harden PYBIND11_MODULE_PYINIT and get_internals() against crashes during module init (#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 gh-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 70b6fd3 commit e8cead1

File tree

2 files changed

+26
-14
lines changed

2 files changed

+26
-14
lines changed

include/pybind11/detail/common.h

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -457,19 +457,24 @@ PyModuleDef_Init should be treated like any other PyObject (so not shared across
457457
static int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject *); \
458458
PYBIND11_PLUGIN_IMPL(name) { \
459459
PYBIND11_CHECK_PYTHON_VERSION \
460-
pybind11::detail::ensure_internals(); \
461-
static ::pybind11::detail::slots_array mod_def_slots = ::pybind11::detail::init_slots( \
462-
&PYBIND11_CONCAT(pybind11_exec_, name), ##__VA_ARGS__); \
463-
static PyModuleDef def{/* m_base */ PyModuleDef_HEAD_INIT, \
464-
/* m_name */ PYBIND11_TOSTRING(name), \
465-
/* m_doc */ nullptr, \
466-
/* m_size */ 0, \
467-
/* m_methods */ nullptr, \
468-
/* m_slots */ mod_def_slots.data(), \
469-
/* m_traverse */ nullptr, \
470-
/* m_clear */ nullptr, \
471-
/* m_free */ nullptr}; \
472-
return PyModuleDef_Init(&def); \
460+
try { \
461+
pybind11::detail::ensure_internals(); \
462+
static ::pybind11::detail::slots_array mod_def_slots \
463+
= ::pybind11::detail::init_slots(&PYBIND11_CONCAT(pybind11_exec_, name), \
464+
##__VA_ARGS__); \
465+
static PyModuleDef def{/* m_base */ PyModuleDef_HEAD_INIT, \
466+
/* m_name */ PYBIND11_TOSTRING(name), \
467+
/* m_doc */ nullptr, \
468+
/* m_size */ 0, \
469+
/* m_methods */ nullptr, \
470+
/* m_slots */ mod_def_slots.data(), \
471+
/* m_traverse */ nullptr, \
472+
/* m_clear */ nullptr, \
473+
/* m_free */ nullptr}; \
474+
return PyModuleDef_Init(&def); \
475+
} \
476+
PYBIND11_CATCH_INIT_EXCEPTIONS \
477+
return nullptr; \
473478
}
474479

475480
#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
@@ -862,14 +862,21 @@ inline internals_pp_manager<internals> &get_internals_pp_manager() {
862862
/// Return a reference to the current `internals` data
863863
PYBIND11_NOINLINE internals &get_internals() {
864864
auto &ppmgr = get_internals_pp_manager();
865-
auto &internals_ptr = *ppmgr.get_pp();
865+
auto *pp = ppmgr.get_pp();
866+
if (!pp) {
867+
pybind11_fail("get_internals: get_pp() returned nullptr");
868+
}
869+
auto &internals_ptr = *pp;
866870
if (!internals_ptr) {
867871
// Slow path, something needs fetched from the state dict or created
868872
gil_scoped_acquire_simple gil;
869873
error_scope err_scope;
870874

871875
ppmgr.create_pp_content_once(&internals_ptr);
872876

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

0 commit comments

Comments
 (0)