Skip to content

Commit c77befe

Browse files
ZeroIntensityTaiju Yamada
authored andcommitted
pythongh-131185: Use a proper thread-local for cached thread states (pythongh-132510)
Switches over to a _Py_thread_local in place of autoTssKey, and also fixes a few other checks regarding PyGILState_Ensure after finalization. Note that this doesn't fix concurrent use of PyGILState_Ensure with Py_Finalize; I'm pretty sure zapthreads doesn't work at all, and that needs to be fixed seperately.
1 parent ebf955d commit c77befe

File tree

7 files changed

+79
-164
lines changed

7 files changed

+79
-164
lines changed

Include/internal/pycore_runtime_init.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,6 @@ extern PyTypeObject _PyExc_MemoryError;
6161
}, \
6262
}, \
6363
}, \
64-
/* A TSS key must be initialized with Py_tss_NEEDS_INIT \
65-
in accordance with the specification. */ \
66-
.autoTSSkey = Py_tss_NEEDS_INIT, \
6764
.parser = _parser_runtime_state_INIT, \
6865
.ceval = { \
6966
.pending_mainthread = { \
@@ -236,4 +233,4 @@ extern PyTypeObject _PyExc_MemoryError;
236233
#ifdef __cplusplus
237234
}
238235
#endif
239-
#endif /* !Py_INTERNAL_RUNTIME_INIT_H */
236+
#endif /* !Py_INTERNAL_RUNTIME_INIT_H */

Include/internal/pycore_runtime_structs.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,6 @@ struct pyruntimestate {
223223
struct _pythread_runtime_state threads;
224224
struct _signals_runtime_state signals;
225225

226-
/* Used for the thread state bound to the current thread. */
227-
Py_tss_t autoTSSkey;
228-
229226
/* Used instead of PyThreadState.trash when there is not current tstate. */
230227
Py_tss_t trashTSSkey;
231228

Lib/test/test_embed.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,6 +1916,10 @@ def test_get_incomplete_frame(self):
19161916
self.run_embedded_interpreter("test_get_incomplete_frame")
19171917

19181918

1919+
def test_gilstate_after_finalization(self):
1920+
self.run_embedded_interpreter("test_gilstate_after_finalization")
1921+
1922+
19191923
class MiscTests(EmbeddingTestsMixin, unittest.TestCase):
19201924
def test_unicode_id_init(self):
19211925
# bpo-42882: Test that _PyUnicode_FromId() works
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:c:func:`PyGILState_Ensure` no longer crashes when called after interpreter
2+
finalization.

Programs/_testembed.c

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <Python.h>
99
#include "pycore_initconfig.h" // _PyConfig_InitCompatConfig()
1010
#include "pycore_runtime.h" // _PyRuntime
11+
#include "pycore_lock.h" // PyEvent
1112
#include "pycore_pythread.h" // PyThread_start_joinable_thread()
1213
#include "pycore_import.h" // _PyImport_FrozenBootstrap
1314
#include <inttypes.h>
@@ -2351,6 +2352,32 @@ test_get_incomplete_frame(void)
23512352
return result;
23522353
}
23532354

2355+
static void
2356+
do_gilstate_ensure(void *event_ptr)
2357+
{
2358+
PyEvent *event = (PyEvent *)event_ptr;
2359+
// Signal to the calling thread that we've started
2360+
_PyEvent_Notify(event);
2361+
PyGILState_Ensure(); // This should hang
2362+
assert(NULL);
2363+
}
2364+
2365+
static int
2366+
test_gilstate_after_finalization(void)
2367+
{
2368+
_testembed_initialize();
2369+
Py_Finalize();
2370+
PyThread_handle_t handle;
2371+
PyThread_ident_t ident;
2372+
PyEvent event = {0};
2373+
if (PyThread_start_joinable_thread(&do_gilstate_ensure, &event, &ident, &handle) < 0) {
2374+
return -1;
2375+
}
2376+
PyEvent_Wait(&event);
2377+
// We're now pretty confident that the thread went for
2378+
// PyGILState_Ensure(), but that means it got hung.
2379+
return PyThread_detach_thread(handle);
2380+
}
23542381

23552382
/* *********************************************************
23562383
* List of test cases and the function that implements it.
@@ -2441,7 +2468,7 @@ static struct TestCase TestCases[] = {
24412468
{"test_frozenmain", test_frozenmain},
24422469
#endif
24432470
{"test_get_incomplete_frame", test_get_incomplete_frame},
2444-
2471+
{"test_gilstate_after_finalization", test_gilstate_after_finalization},
24452472
{NULL, NULL}
24462473
};
24472474

0 commit comments

Comments
 (0)