8000 gh-115035: Mark ThreadHandles as non-joinable earlier after forking (… · python/cpython@b6228b5 · GitHub
[go: up one dir, main page]
Skip to content

Commit b6228b5

Browse files
colesburycolesbury
authored
gh-115035: Mark ThreadHandles as non-joinable earlier after forking (#115042)
This marks dead ThreadHandles as non-joinable earlier in `PyOS_AfterFork_Child()` before we execute any Python code. The handles are stored in a global linked list in `_PyRuntimeState` because `fork()` affects the entire process.
1 parent 71239d5 commit b6228b5

File tree

7 files changed

+50
-41
lines changed

7 files changed

+50
-41
lines changed

‎Include/internal/pycore_pythread.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ extern "C" {
99
#endif
1010

1111
#include "dynamic_annotations.h" // _Py_ANNOTATE_PURE_HAPPENS_BEFORE_MUTEX
12+
#include "pycore_llist.h" // struct llist_node
1213

1314
// Get _POSIX_THREADS and _POSIX_SEMAPHORES macros if available
1415
#if (defined(HAVE_UNISTD_H) && !defined(_POSIX_THREADS) \
@@ -75,14 +76,22 @@ struct _pythread_runtime_state {
75 8000 76
struct py_stub_tls_entry tls_entries[PTHREAD_KEYS_MAX];
7677
} stubs;
7778
#endif
79+
80+
// Linked list of ThreadHandleObjects
81+
struct llist_node handles;
7882
};
7983

84+
#define _pythread_RUNTIME_INIT(pythread) \
85+
{ \
86+
.handles = LLIST_INIT(pythread.handles), \
87+
}
8088

8189
#ifdef HAVE_FORK
8290
/* Private function to reinitialize a lock at fork in the child process.
8391
Reset the lock to the unlocked state.
8492
Return 0 on success, return -1 on error. */
8593
extern int _PyThread_at_fork_reinit(PyThread_type_lock *lock);
94+
extern void _PyThread_AfterFork(struct _pythread_runtime_state *state);
8695
#endif /* HAVE_FORK */
< 8000 /td>
8796

8897

@@ -143,12 +152,6 @@ PyAPI_FUNC(int) PyThread_join_thread(PyThread_handle_t);
143152
*/
144153
PyAPI_FUNC(int) PyThread_detach_thread(PyThread_handle_t);
145154

146-
/*
147-
* Obtain the new thread ident and handle in a forked child process.
148-
*/
149-
PyAPI_FUNC(void) PyThread_update_thread_after_fork(PyThread_ident_t* ident,
150-
PyThread_handle_t* handle);
151-
152155
#ifdef __cplusplus
153156
}
154157
#endif

‎Include/internal/pycore_runtime_init.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ extern "C" {
1616
#include "pycore_parser.h" // _parser_runtime_state_INIT
1717
#include "pycore_pyhash.h" // pyhash_state_INIT
1818
#include "pycore_pymem_init.h" // _pymem_allocators_standard_INIT
19+
#include "pycore_pythread.h" // _pythread_RUNTIME_INIT
1920
#include "pycore_runtime_init_generated.h" // _Py_bytes_characters_INIT
2021
#include "pycore_signal.h" // _signals_RUNTIME_INIT
2122
#include "pycore_tracemalloc.h" // _tracemalloc_runtime_state_INIT
@@ -90,6 +91,7 @@ extern PyTypeObject _PyExc_MemoryError;
9091
}, \
9192
.obmalloc = _obmalloc_global_state_INIT, \
9293
.pyhash_state = pyhash_state_INIT, \
94+
.threads = _pythread_RUNTIME_INIT(runtime.threads), \
9395
.signals = _signals_RUNTIME_INIT, \
9496
.interpreters = { \
9597
/* This prevents interpreters from getting created \

‎Lib/threading.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -949,7 +949,6 @@ def _after_fork(self, new_ident=None):
949949
# This thread is alive.
950950
self._ident = new_ident
951951
if self._handle is not None:
952-
self._handle.after_fork_alive()
953952
assert self._handle.ident == new_ident
954953
# bpo-42350: If the fork happens when the thread is already stopped
955954
# (ex: after threading._shutdown() has been called), _tstate_lock
@@ -965,9 +964,7 @@ def _after_fork(self, new_ident=None):
965964
self._is_stopped = True
966965
self._tstate_lock = None
967966
self._join_lock = None
968-
if self._handle is not None:
969-
self._handle.after_fork_dead()
970-
self._handle = None
967+
self._handle = None
971968

972969
def __repr__(self):
973970
assert self._initialized, "Thread.__init__() was not called"

‎Modules/_threadmodule.c

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ get_thread_state(PyObject *module)
4444

4545
typedef struct {
4646
PyObject_HEAD
47+
struct llist_node node; // linked list node (see _pythread_runtime_state)
4748
PyThread_ident_t ident;
4849
PyThread_handle_t handle;
4950
char joinable;
@@ -59,13 +60,26 @@ new_thread_handle(thread_module_state* state)
5960
self->ident = 0;
6061
self->handle = 0;
6162
self->joinable = 0;
63+
64+
HEAD_LOCK(&_PyRuntime);
65+
llist_insert_tail(&_PyRuntime.threads.handles, &self->node);
66+
HEAD_UNLOCK(&_PyRuntime);
67+
6268
return self;
6369
}
6470

6571
static void
6672
ThreadHandle_dealloc(ThreadHandleObject *self)
6773
{
6874
PyObject *tp = (PyObject *) Py_TYPE(self);
75+
76+
// Remove ourself from the global list of handles
77+
HEAD_LOCK(&_PyRuntime);
78+
if (self->node.next != NULL) {
79+
llist_remove(&self->node);
80+
}
81+
HEAD_UNLOCK(&_PyRuntime);
82+
6983
if (self->joinable) {
7084
int ret = PyThread_detach_thread(self->handle);
7185
if (ret) {
@@ -77,6 +91,28 @@ ThreadHandle_dealloc(ThreadHandleObject *self)
7791
Py_DECREF(tp);
7892
}
7993

94+
void
95+
_PyThread_AfterFork(struct _pythread_runtime_state *state)
96+
{
97+
// gh-115035: We mark ThreadHandles as not joinable early in the child's
98+
// after-fork handler. We do this before calling any Python code to ensure
99+
// that it happens before any ThreadHandles are deallocated, such as by a
100+
// GC cycle.
101+
PyThread_ident_t current = PyThread_get_thread_ident_ex();
102+
103+
struct llist_node *node;
104+
llist_for_each_safe(node, &state->handles) {
105+
ThreadHandleObject *hobj = llist_data(node, ThreadHandleObject, node);
106+
if (hobj->ident == current) {
107+
continue;
108+
}
109+
110+
// Disallow calls to detach() and join() as they could crash.
111+
hobj->joinable = 0;
112+
llist_remove(node);
113+
}
114+
}
115+
80116
static PyObject *
81117
ThreadHandle_repr(ThreadHandleObject *self)
82118
{
@@ -91,21 +127,6 @@ ThreadHandle_get_ident(ThreadHandleObject *self, void *ignored)
91127
}
92128

93129

94-
static PyObject *
95-
ThreadHandle_after_fork_alive(ThreadHandleObject *self, void* ignored)
96-
{
97-
PyThread_update_thread_after_fork(&self->ident, &self->handle);
98-
Py_RETURN_NONE;
99-
}
100-
101-
static PyObject *
102-
ThreadHandle_after_fork_dead(ThreadHandleObject *self, void* ignored)
103-
{
104-
// Disallow calls to detach() and join() as they could crash.
105-
self->joinable = 0;
106-
Py_RETURN_NONE;
107-
}
108-
109130
static PyObject *
110131
ThreadHandle_detach(ThreadHandleObject *self, void* ignored)
111132
{
@@ -157,8 +178,6 @@ static PyGetSetDef ThreadHandle_getsetlist[] = {
157178

158179
static PyMethodDef ThreadHandle_methods[] =
159180
{
160-
{"after_fork_alive", (PyCFunction)ThreadHandle_after_fork_alive, METH_NOARGS},
161-
{"after_fork_dead", (PyCFunction)ThreadHandle_after_fork_dead, METH_NOARGS},
162181
{"detach", (PyCFunction)ThreadHandle_detach, METH_NOARGS},
163182
{"join", (PyCFunction)ThreadHandle_join, METH_NOARGS},
164183
{0, 0}

‎Python/pystate.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,8 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime)
517517
return _PyStatus_NO_MEMORY();
518518
}
519519

520+
_PyThread_AfterFork(&runtime->threads);
521+
520522
return _PyStatus_OK();
521523
}
522524
#endif

‎Python/thread_nt.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,6 @@ PyThread_detach_thread(PyThread_handle_t handle) {
242242
return (CloseHandle(hThread) == 0);
243243
}
244244

245-
void
246-
PyThread_update_thread_after_fork(PyThread_ident_t* ident, PyThread_handle_t* handle) {
247-
}
248-
249245
/*
250246
* Return the thread Id instead of a handle. The Id is said to uniquely identify the
251247
* thread in the system

‎Python/thread_pthread.h

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -339,16 +339,6 @@ PyThread_detach_thread(PyThread_handle_t th) {
339339
return pthread_detach((pthread_t) th);
340340
}
341341

342-
void
343-
PyThread_update_thread_after_fork(PyThread_ident_t* ident, PyThread_handle_t* handle) {
344-
// The thread id might have been updated in the forked child
345-
pthread_t th = pthread_self();
346-
*ident = (PyThread_ident_t) th;
347-
*handle = (PyThread_handle_t) th;
348-
assert(th == (pthread_t) *ident);
349-
assert(th == (pthread_t) *handle);
350-
}
351-
352342
/* XXX This implementation is considered (to quote Tim Peters) "inherently
353343
hosed" because:
354344
- It does not guarantee the promise that a non-zero integer is returned.

0 commit comments

Comments
 (0)
0