Skip to content

Commit 52c1c30

Browse files
committed
I think this fixes the EndInterpreter issues on all versions.
It just has to be ifdef'd because it is slightly broken on 3.12, working well on 3.13, and kind of crashy on 3.14beta. These two verion ifdefs solve all the issues.
1 parent c5170f8 commit 52c1c30

File tree

1 file changed

+61
-59
lines changed

1 file changed

+61
-59
lines changed

include/pybind11/subinterpreter.h

Lines changed: 61 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,15 @@ class subinterpreter {
6161
subinterpreter(subinterpreter const &copy) = delete;
6262
subinterpreter &operator=(subinterpreter const &copy) = delete;
6363

64-
subinterpreter(subinterpreter &&old) : tstate_(old.tstate_), istate_(old.istate_) {
65-
old.tstate_ = nullptr;
64+
subinterpreter(subinterpreter &&old)
65+
: istate_(old.istate_), creation_tstate_(old.creation_tstate_) {
6666
old.istate_ = nullptr;
67+
old.creation_tstate_ = nullptr;
6768
}
6869

6970
subinterpreter &operator=(subinterpreter &&old) {
70-
std::swap(old.tstate_, tstate_);
7171
std::swap(old.istate_, istate_);
72+
std::swap(old.creation_tstate_, creation_tstate_);
7273
return *this;
7374
}
7475

@@ -85,18 +86,26 @@ class subinterpreter {
8586

8687
auto prev_tstate = PyThreadState_Get();
8788

88-
auto status = Py_NewInterpreterFromConfig(&result.tstate_, &cfg);
89+
auto status = Py_NewInterpreterFromConfig(&result.creation_tstate_, &cfg);
8990

9091
// this doesn't raise a normal Python exception, it provides an exit() status code.
9192
if (PyStatus_Exception(status)) {
9293
pybind11_fail("failed to create new sub-interpreter");
9394
}
9495

9596
// upon success, the new interpreter is activated in this thread
96-
result.istate_ = result.tstate_->interp;
97+
result.istate_ = result.creation_tstate_->interp;
9798
detail::get_num_interpreters_seen() += 1; // there are now many interpreters
9899
detail::get_internals(); // initialize internals.tstate, amongst other things...
99100

101+
// In 3.13+ this state should be deleted right away, and the memory will be reused for
102+
// the next threadstate on this interpreter. However, on 3.12 we cannot do that, we
103+
// must keep it around (but not use it) ... see destructor.
104+
#if PY_VERSION_HEX >= 0x030D0000
105+
PyThreadState_Clear(result.creation_tstate_);
106+
PyThreadState_DeleteCurrent();
107+
#endif
108+
100109
// we have to switch back to main, and then the scopes will handle cleanup
101110
PyThreadState_Swap(prev_tstate);
102111
}
@@ -115,65 +124,58 @@ class subinterpreter {
115124
}
116125

117126
~subinterpreter() {
118-
if (tstate_) {
119-
PyThreadState *old_tstate;
127+
if (!creation_tstate_) {
128+
// non-owning wrapper, do nothing.
129+
return;
130+
}
120131

121-
bool wrong_thread = true;
122-
#ifdef PY_HAVE_THREAD_NATIVE_ID
123-
wrong_thread = PyThread_get_thread_native_id() != tstate_->native_thread_id;
124-
#endif
125-
if (wrong_thread) {
126-
// The PyThreadState used in Py_EndInterpreter must be created on this OS thread.
127-
// (If it is not, subinterpreter cleanup will hang within Python's threading
128-
// module.) So create a new thread state here on the right OS thread.
129-
auto *temp = PyThreadState_New(tstate_->interp);
130-
old_tstate = PyThreadState_Swap(temp);
131-
// delete the other one because there must be only one at cleanup
132-
PyThreadState_Clear(tstate_);
133-
PyThreadState_Delete(tstate_);
134-
tstate_ = temp;
135-
} else {
136-
old_tstate = PyThreadState_Swap(tstate_);
137-
}
132+
auto *temp = PyThreadState_New(istate_);
133+
auto *old_tstate = PyThreadState_Swap(temp);
138134

139-
bool switch_back = old_tstate && old_tstate->interp != tstate_->interp;
140-
141-
// make sure we have the GIL for the interpreter we are ending
142-
(void) PyGILState_Ensure();
143-
144-
// Get the internals pointer (without creating it if it doesn't exist). It's possible
145-
// for the internals to be created during Py_EndInterpreter() (e.g. if a py::capsule
146-
// calls `get_internals()` during destruction), so we get the pointer-pointer here and
147-
// check it after.
148-
auto *&internals_ptr_ptr = detail::get_internals_pp<detail::internals>();
149-
auto *&local_internals_ptr_ptr = detail::get_internals_pp<detail::local_internals>();
150-
{
151-
dict sd = state_dict();
152-
internals_ptr_ptr
153-
= detail::get_internals_pp_from_capsule_in_state_dict<detail::internals>(
154-
sd, PYBIND11_INTERNALS_ID);
155-
local_internals_ptr_ptr
156-
= detail::get_internals_pp_from_capsule_in_state_dict<detail::local_internals>(
157-
sd, detail::get_local_internals_id());
158-
}
135+
bool switch_back = old_tstate && old_tstate->interp != istate_;
159136

160-
// End it
161-
Py_EndInterpreter(tstate_);
137+
#if PY_VERSION_HEX < 0x030D0000
138+
// In 3.12 we can only delete this thread state when we know we are about to End the
139+
// interpreter... because otherwise Python will try to reuse it, and fail, and abort. We
140+
// cannot use this one in the call to EndInterpreter either because it may have been
141+
// created on a different OS thread. So we have to make a new one first, switch to that
142+
// one, and the delete this one finally.
143+
PyThreadState_Clear(creation_tstate_);
144+
PyThreadState_Delete(creation_tstate_);
145+
#endif
162146

163-
// do NOT decrease detail::get_num_interpreters_seen, because it can never decrease
164-
// while other threads are running...
147+
// Get the internals pointer (without creating it if it doesn't exist). It's possible
148+
// for the internals to be created during Py_EndInterpreter() (e.g. if a py::capsule
149+
// calls `get_internals()` during destruction), so we get the pointer-pointer here and
150+
// check it after.
151+
auto *&internals_ptr_ptr = detail::get_internals_pp<detail::internals>();
152+
auto *&local_internals_ptr_ptr = detail::get_internals_pp<detail::local_internals>();
153+
{
154+
dict sd = state_dict();
155+
internals_ptr_ptr
156+
= detail::get_internals_pp_from_capsule_in_state_dict<detail::internals>(
157+
sd, PYBIND11_INTERNALS_ID);
158+
local_internals_ptr_ptr
159+
= detail::get_internals_pp_from_capsule_in_state_dict<detail::local_internals>(
160+
sd, detail::get_local_internals_id());
161+
}
165162

166-
if (internals_ptr_ptr) {
167-
internals_ptr_ptr->reset();
168-
}
169-
if (local_internals_ptr_ptr) {
170-
local_internals_ptr_ptr->reset();
171-
}
163+
// End it
164+
Py_EndInterpreter(temp);
172165

173-
// switch back to the old tstate and old GIL (if there was one)
174-
if (switch_back)
175-
PyThreadState_Swap(old_tstate);
166+
// do NOT decrease detail::get_num_interpreters_seen, because it can never decrease
167+
// while other threads are running...
168+
169+
if (internals_ptr_ptr) {
170+
internals_ptr_ptr->reset();
171+
}
172+
if (local_internals_ptr_ptr) {
173+
local_internals_ptr_ptr->reset();
176174
}
175+
176+
// switch back to the old tstate and old GIL (if there was one)
177+
if (switch_back)
178+
PyThreadState_Swap(old_tstate);
177179
}
178180

179181
/// Get a handle to the main interpreter that can be used with subinterpreter_scoped_activate
@@ -208,7 +210,7 @@ class subinterpreter {
208210

209211
/// abandon cleanup of this subinterpreter (leak it). this might be needed during
210212
/// finalization...
211-
void disarm() { tstate_ = nullptr; }
213+
void disarm() { creation_tstate_ = nullptr; }
212214

213215
/// An empty wrapper cannot be activated
214216
bool empty() const { return istate_ == nullptr; }
@@ -218,8 +220,8 @@ class subinterpreter {
218220

219221
private:
220222
friend class subinterpreter_scoped_activate;
221-
PyThreadState *tstate_ = nullptr;
222223
PyInterpreterState *istate_ = nullptr;
224+
PyThreadState *creation_tstate_ = nullptr;
223225
};
224226

225227
class scoped_subinterpreter {

0 commit comments

Comments
 (0)