Skip to content

Commit 03b4a9e

Browse files
b-passb-pass
authored
fix: don't destruct module objects in atexit (#5688)
1 parent 1dd85ef commit 03b4a9e

File tree

3 files changed

+61
-94
lines changed

3 files changed

+61
-94
lines changed

‎include/pybind11/detail/common.h‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -385,35 +385,39 @@
385385
} \
386386
PyObject *pybind11_init()
387387

388+
// this push is for the next several macros
388389
PYBIND11_WARNING_PUSH
389390
PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")
391+
392+
/**
393+
Create a PyInit_ function for this module.
394+
395+
Note that this is run once for each (sub-)interpreter the module is imported into, including
396+
possibly concurrently. The PyModuleDef is allowed to be static, but the PyObject* resulting from
397+
PyModuleDef_Init should be treated like any other PyObject (so not shared across interpreters).
398+
*/
390399
#define PYBIND11_MODULE_PYINIT(name, pre_init, ...) \
391-
static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name); \
392-
static ::pybind11::module_::slots_array PYBIND11_CONCAT(pybind11_module_slots_, name); \
393400
static int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject *); \
394-
static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \
395401
PYBIND11_PLUGIN_IMPL(name) { \
396402
PYBIND11_CHECK_PYTHON_VERSION \
397403
pre_init; \
398404
PYBIND11_ENSURE_INTERNALS_READY \
399-
static auto result = []() { \
400-
auto &slots = PYBIND11_CONCAT(pybind11_module_slots_, name); \
401-
slots[0] = {Py_mod_exec, \
402-
reinterpret_cast<void *>(&PYBIND11_CONCAT(pybind11_exec_, name))}; \
403-
slots[1] = {0, nullptr}; \
404-
return ::pybind11::module_::initialize_multiphase_module_def( \
405-
PYBIND11_TOSTRING(name), \
406-
nullptr, \
407-
&PYBIND11_CONCAT(pybind11_module_def_, name), \
408-
slots, \
409-
##__VA_ARGS__); \
410-
}(); \
411-
return result.ptr(); \
405+
static ::pybind11::detail::slots_array slots = ::pybind11::detail::init_slots( \
406+
&PYBIND11_CONCAT(pybind11_exec_, name), ##__VA_ARGS__); \
407+
static PyModuleDef def{/* m_base */ PyModuleDef_HEAD_INIT, \
408+
/* m_name */ PYBIND11_TOSTRING(name), \
409+
/* m_doc */ nullptr, \
410+
/* m_size */ 0, \
411+
/* m_methods */ nullptr, \
412+
/* m_slots */ slots.data(), \
413+
/* m_traverse */ nullptr, \
414+
/* m_clear */ nullptr, \
415+
/* m_free */ nullptr}; \
416+
return PyModuleDef_Init(&def); \
412417
}
413418

414-
PYBIND11_WARNING_POP
415-
416419
#define PYBIND11_MODULE_EXEC(name, variable) \
420+
static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \
417421
int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject * pm) { \
418422
try { \
419423
auto m = pybind11::reinterpret_borrow<::pybind11::module_>(pm); \
@@ -464,12 +468,12 @@ PYBIND11_WARNING_POP
464468
}
465469
466470
\endrst */
467-
PYBIND11_WARNING_PUSH
468-
PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")
469471
#define PYBIND11_MODULE(name, variable, ...) \
470472
PYBIND11_MODULE_PYINIT( \
471473
name, (pybind11::detail::get_num_interpreters_seen() += 1), ##__VA_ARGS__) \
472474
PYBIND11_MODULE_EXEC(name, variable)
475+
476+
// pop gnu-zero-variadic-macro-arguments
473477
PYBIND11_WARNING_POP
474478

475479
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)

‎include/pybind11/pybind11.h‎

Lines changed: 36 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1325,6 +1325,38 @@ inline void *multi_interp_slot(F &&, O &&...o) {
13251325
}
13261326
#endif
13271327

1328+
/// Must be a POD type, and must hold enough entries for all of the possible slots PLUS ONE for
1329+
/// the sentinel (0) end slot.
1330+
using slots_array = std::array<PyModuleDef_Slot, 4>;
1331+
1332+
/// Initialize an array of slots based on the supplied exec slot and options.
1333+
template <typename... Options>
1334+
static slots_array init_slots(int (*exec_fn)(PyObject *), Options &&...options) noexcept {
1335+
/* NOTE: slots_array MUST be large enough to hold all possible options. If you add an option
1336+
here, you MUST also increase the size of slots_array in the type alias above! */
1337+
slots_array slots;
1338+
size_t next_slot = 0;
1339+
1340+
if (exec_fn != nullptr) {
1341+
slots[next_slot++] = {Py_mod_exec, reinterpret_cast<void *>(exec_fn)};
1342+
}
1343+
1344+
#ifdef Py_mod_multiple_interpreters
1345+
slots[next_slot++] = {Py_mod_multiple_interpreters, multi_interp_slot(options...)};
1346+
#endif
1347+
1348+
if (gil_not_used_option(options...)) {
1349+
#if defined(Py_mod_gil) && defined(Py_GIL_DISABLED)
1350+
slots[next_slot++] = {Py_mod_gil, Py_MOD_GIL_NOT_USED};
1351+
#endif
1352+
}
1353+
1354+
// slots must have a zero end sentinel
1355+
slots[next_slot++] = {0, nullptr};
1356+
1357+
return slots;
1358+
}
1359+
13281360
PYBIND11_NAMESPACE_END(detail)
13291361

13301362
/// Wrapper for Python extension modules
@@ -1438,19 +1470,19 @@ class module_ : public object {
14381470
PyModule_AddObject(ptr(), name, obj.inc_ref().ptr() /* steals a reference */);
14391471
}
14401472

1441-
using module_def = PyModuleDef; // TODO: Can this be removed (it was needed only for Python 2)?
1473+
// DEPRECATED (since PR #5688): Use PyModuleDef directly instead.
1474+
using module_def = PyModuleDef;
14421475

14431476
/** \rst
14441477
Create a new top-level module that can be used as the main module of a C extension.
14451478
1446-
``def`` should point to a statically allocated module_def.
1479+
``def`` should point to a statically allocated PyModuleDef.
14471480
\endrst */
14481481
static module_ create_extension_module(const char *name,
14491482
const char *doc,
1450-
module_def *def,
1483+
PyModuleDef *def,
14511484
mod_gil_not_used gil_not_used
14521485
= mod_gil_not_used(false)) {
1453-
// module_def is PyModuleDef
14541486
// Placement new (not an allocation).
14551487
new (def) PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT,
14561488
/* m_name */ name,
@@ -1478,74 +1510,6 @@ class module_ : public object {
14781510
// For Python 2, reinterpret_borrow was correct.
14791511
return reinterpret_borrow<module_>(m);
14801512
}
1481-
1482-
/// Must be a POD type, and must hold enough entries for all of the possible slots PLUS ONE for
1483-
/// the sentinel (0) end slot.
1484-
using slots_array = std::array<PyModuleDef_Slot, 4>;
1485-
1486-
/** \rst
1487-
Initialize a module def for use with multi-phase module initialization.
1488-
1489-
``def`` should point to a statically allocated module_def.
1490-
``slots`` must already contain a Py_mod_exec or Py_mod_create slot and will be filled with
1491-
additional slots from the supplied options (and the empty sentinel slot).
1492-
\endrst */
1493-
template <typename... Options>
1494-
static object initialize_multiphase_module_def(const char *name,
1495-
const char *doc,
1496-
module_def *def,
1497-
slots_array &slots,
1498-
Options &&...options) {
1499-
size_t next_slot = 0;
1500-
size_t term_slot = slots.size() - 1;
1501-
1502-
// find the end of the supplied slots
1503-
while (next_slot < term_slot && slots[next_slot].slot != 0) {
1504-
++next_slot;
1505-
}
1506-
1507-
#ifdef Py_mod_multiple_interpreters
1508-
if (next_slot >= term_slot) {
1509-
pybind11_fail("initialize_multiphase_module_def: not enough space in slots");
1510-
}
1511-
slots[next_slot++] = {Py_mod_multiple_interpreters, detail::multi_interp_slot(options...)};
1512-
#endif
1513-
1514-
if (detail::gil_not_used_option(options...)) {
1515-
#if defined(Py_mod_gil) && defined(Py_GIL_DISABLED)
1516-
if (next_slot >= term_slot) {
1517-
pybind11_fail("initialize_multiphase_module_def: not enough space in slots");
1518-
}
1519-
slots[next_slot++] = {Py_mod_gil, Py_MOD_GIL_NOT_USED};
1520-
#endif
1521-
}
1522-
1523-
// slots must have a zero end sentinel
1524-
if (next_slot > term_slot) {
1525-
pybind11_fail("initialize_multiphase_module_def: not enough space in slots");
1526-
}
1527-
slots[next_slot++] = {0, nullptr};
1528-
1529-
// module_def is PyModuleDef
1530-
// Placement new (not an allocation).
1531-
new (def) PyModuleDef{/* m_base */ PyModuleDef_HEAD_INIT,
1532-
/* m_name */ name,
1533-
/* m_doc */ options::show_user_defined_docstrings() ? doc : nullptr,
1534-
/* m_size */ 0,
1535-
/* m_methods */ nullptr,
1536-
/* m_slots */ &slots[0],
1537-
/* m_traverse */ nullptr,
1538-
/* m_clear */ nullptr,
1539-
/* m_free */ nullptr};
1540-
auto *m = PyModuleDef_Init(def);
1541-
if (m == nullptr) {
1542-
if (PyErr_Occurred()) {
1543-
throw error_already_set();
1544-
}
1545-
pybind11_fail("Internal error in module_::initialize_multiphase_module_def()");
1546-
}
1547-
return reinterpret_borrow<object>(m);
1548-
}
15491513
};
15501514

15511515
PYBIND11_NAMESPACE_BEGIN(detail)

‎tests/test_modules.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,7 @@ TEST_SUBMODULE(modules, m) {
7878
class DupeException {};
7979

8080
// Go ahead and leak, until we have a non-leaking py::module_ constructor
81-
auto dm
82-
= py::module_::create_extension_module("dummy", nullptr, new py::module_::module_def);
81+
auto dm = py::module_::create_extension_module("dummy", nullptr, new PyModuleDef);
8382
auto failures = py::list();
8483

8584
py::class_<Dupe1>(dm, "Dupe1");

0 commit comments

Comments
 (0)