Skip to content

Commit 89e9599

Browse files
committed
Wrap module_def initialization in a static so it only happens once.
If it happens concurrently in multiple threads, badness ensues....
1 parent 22968e9 commit 89e9599

File tree

2 files changed

+17
-23
lines changed

2 files changed

+17
-23
lines changed

include/pybind11/detail/common.h

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -401,17 +401,19 @@ PYBIND11_WARNING_DISABLE_CLANG("-Wgnu-zero-variadic-macro-arguments")
401401
PYBIND11_PLUGIN_IMPL(name) { \
402402
PYBIND11_CHECK_PYTHON_VERSION \
403403
PYBIND11_ENSURE_INTERNALS_READY \
404-
auto &slots = PYBIND11_CONCAT(pybind11_module_slots_, name); \
405-
slots[0] \
406-
= {Py_mod_exec, reinterpret_cast<void *>(&PYBIND11_CONCAT(pybind11_exec_, name))}; \
407-
slots[1] = {0, nullptr}; \
408-
auto m = ::pybind11::module_::initialize_multiphase_module_def( \
409-
PYBIND11_TOSTRING(name), \
410-
nullptr, \
411-
&PYBIND11_CONCAT(pybind11_module_def_, name), \
412-
slots, \
413-
##__VA_ARGS__); \
414-
return m.ptr(); \
404+
static auto result = []() { \
405+
auto &slots = PYBIND11_CONCAT(pybind11_module_slots_, name); \
406+
slots[0] = {Py_mod_exec, \
407+
reinterpret_cast<void *>(&PYBIND11_CONCAT(pybind11_exec_, name))}; \
408+
slots[1] = {0, nullptr}; \
409+
return ::pybind11::module_::initialize_multiphase_module_def( \
410+
PYBIND11_TOSTRING(name), \
411+
nullptr, \
412+
&PYBIND11_CONCAT(pybind11_module_def_, name), \
413+
slots, \
414+
##__VA_ARGS__); \
415+
}(); \
416+
return result.ptr(); \
415417
} \
416418
int PYBIND11_CONCAT(pybind11_exec_, name)(PyObject * pm) { \
417419
pybind11::detail::get_num_interpreters_seen() += 1; \

tests/test_embed/test_interpreter.cpp

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -465,10 +465,9 @@ TEST_CASE("Per-Subinterpreter GIL") {
465465
auto main_int
466466
= py::module_::import("external_module").attr("internals_at")().cast<uintptr_t>();
467467

468-
std::atomic<int> started, sync, finished, failure;
468+
std::atomic<int> started, sync, failure;
469469
started = 0;
470470
sync = 0;
471-
finished = 0;
472471
failure = 0;
473472

474473
// REQUIRE throws on failure, so we can't use it within the thread
@@ -541,9 +540,7 @@ TEST_CASE("Per-Subinterpreter GIL") {
541540
T_REQUIRE(sub_int != main_int);
542541

543542
Py_EndInterpreter(sub);
544-
545-
++finished;
546-
543+
547544
PyThreadState_Swap(
548545
main_tstate); // switch back so the scoped_acquire can release the GIL properly
549546
};
@@ -583,16 +580,11 @@ TEST_CASE("Per-Subinterpreter GIL") {
583580
// so we move sync so that thread 2 can finish executing
584581
++sync;
585582

586-
++finished;
587-
588583
// now wait for both threads to complete
589-
while (finished != 3)
590-
std::this_thread::sleep_for(std::chrono::microseconds(1));
584+
t1.join();
585+
t2.join();
591586
}
592587

593-
t1.join();
594-
t2.join();
595-
596588
// now we have the gil again, sanity check
597589
REQUIRE(py::cast<std::string>(py::module_::import("external_module").attr("multi_interp"))
598590
== "1");

0 commit comments

Comments
 (0)