Skip to content

Commit 0556fc6

Browse files
committed
Try fix concurrency by not using get_num_interpreters_seen() > 1
1 parent 3886ac9 commit 0556fc6

File tree

2 files changed

+27
-30
lines changed

2 files changed

+27
-30
lines changed

include/pybind11/detail/common.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,9 @@
359359
#define PYBIND11_ENSURE_INTERNALS_READY \
360360
{ \
361361
pybind11::detail::get_internals_pp_manager().unref(); \
362+
pybind11::detail::get_local_internals_pp_manager().unref(); \
362363
pybind11::detail::get_internals(); \
364+
pybind11::detail::get_local_internals(); \
363365
}
364366

365367
#if !defined(GRAALVM_PYTHON)

include/pybind11/detail/internals.h

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -588,8 +588,7 @@ Payload *atomic_get_or_create_in_state_dict(const char *key) {
588588
// immediately, which will also free the storage.
589589
/*destructor=*/[](void *ptr) -> void { delete static_cast<Payload *>(ptr); });
590590
// At this point, the capsule object is created successfully.
591-
// Release the unique_ptr and let the capsule object own the storage to avoid
592-
// double-free.
591+
// Release the unique_ptr and let the capsule object own the storage to avoid double-free.
593592
(void) storage_ptr.release();
594593

595594
// Use `PyDict_SetDefault` for atomic test-and-set:
@@ -614,10 +613,10 @@ Payload *atomic_get_or_create_in_state_dict(const char *key) {
614613
}
615614
}
616615
PYBIND11_WARNING_POP
617-
// - If key already existed, our `new_capsule` is not inserted, it will be destructed
618-
// when going out of scope here, which will also free the storage.
619-
// - Otherwise, our `new_capsule` is now in the dict, and it owns the storage and the
620-
// state dict will incref it.
616+
// - If key already existed, our `new_capsule` is not inserted, it will be destructed when
617+
// going out of scope here, which will also free the storage.
618+
// - Otherwise, our `new_capsule` is now in the dict, and it owns the storage and the state
619+
// dict will incref it.
621620
}
622621

623622
// Get the storage pointer from the capsule.
@@ -644,27 +643,26 @@ class internals_pp_manager {
644643
/// acquire the GIL. Will never return nullptr.
645644
std::unique_ptr<InternalsType> *get_pp() {
646645
#ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT
647-
if (get_num_interpreters_seen() > 1 || last_istate_tls() == nullptr) {
648-
// Whenever the interpreter changes on the current thread we need to invalidate the
649-
// internals_pp so that it can be pulled from the interpreter's state dict. That is
650-
// slow, so we use the current PyThreadState to check if it is necessary.
651-
auto *tstate = get_thread_state_unchecked();
652-
if (!tstate || tstate->interp != last_istate_tls()) {
653-
gil_scoped_acquire_simple gil;
654-
if (!tstate) {
655-
tstate = get_thread_state_unchecked();
656-
}
657-
last_istate_tls() = tstate->interp;
658-
internals_p_tls() = get_or_create_pp_in_state_dict();
646+
// Whenever the interpreter changes on the current thread we need to invalidate the
647+
// internals_pp so that it can be pulled from the interpreter's state dict. That is
648+
// slow, so we use the current PyThreadState to check if it is necessary.
649+
auto *tstate = get_thread_state_unchecked();
650+
if (!tstate || tstate->interp != last_istate_tls()) {
651+
gil_scoped_acquire_simple gil;
652+
if (!tstate) {
653+
tstate = get_thread_state_unchecked();
659654
}
660-
return internals_p_tls();
655+
last_istate_tls() = tstate->interp;
656+
internals_p_tls() = get_or_create_pp_in_state_dict();
661657
}
662-
#endif
658+
return internals_p_tls();
659+
#else
663660
if (!internals_singleton_pp_) {
664661
gil_scoped_acquire_simple gil;
665662
internals_singleton_pp_ = get_or_create_pp_in_state_dict();
666663
}
667664
return internals_singleton_pp_;
665+
#endif
668666
}
669667

670668
/// Drop all the references we're currently holding.
@@ -678,18 +676,15 @@ class internals_pp_manager {
678676

679677
void destroy() {
680678
#ifdef PYBIND11_HAS_SUBINTERPRETER_SUPPORT
681-
if (get_num_interpreters_seen() > 1) {
682-
auto *tstate = get_thread_state_unchecked();
683-
// this could be called without an active interpreter, just use what was cached
684-
if (!tstate || tstate->interp == last_istate_tls()) {
685-
auto tpp = internals_p_tls();
686-
delete tpp;
687-
}
688-
unref();
689-
return;
679+
auto *tstate = get_thread_state_unchecked();
680+
// this could be called without an active interpreter, just use what was cached
681+
if (!tstate || tstate->interp == last_istate_tls()) {
682+
auto tpp = internals_p_tls();
683+
delete tpp;
690684
}
691-
#endif
685+
#else
692686
delete internals_singleton_pp_;
687+
#endif
693688
unref();
694689
}
695690

0 commit comments

Comments
 (0)