Skip to content

Commit 704645f

Browse files
committed
Ensure fork safety
1 parent 43a1dfb commit 704645f

File tree

5 files changed

+54
-11
lines changed

5 files changed

+54
-11
lines changed

Include/internal/pycore_pythread.h

+6
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,12 @@ PyAPI_FUNC(int) PyThread_join_thread(Py_uintptr_t);
137137
*/
138138
PyAPI_FUNC(int) PyThread_detach_thread(Py_uintptr_t);
139139

140+
/*
141+
* Obtain the new thread ident and handle in a forked child process.
142+
*/
143+
PyAPI_FUNC(void) PyThread_update_thread_after_fork(unsigned long long* ident,
144+
Py_uintptr_t* handle);
145+
140146
#ifdef __cplusplus
141147
}
142148
#endif

Lib/threading.py

+16-11
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,7 @@ def __repr__(self):
590590
return f"<{cls.__module__}.{cls.__qualname__} at {id(self):#x}: {status}>"
591591

592592
def _at_fork_reinit(self):
593-
# Private method called by Thread._reset_internal_locks()
593+
# Private method called by Thread._after_fork()
594594
self._cond._at_fork_reinit()
595595

596596
def is_set(self):
@@ -936,11 +936,15 @@ class is implemented.
936936
# For debugging and _after_fork()
937937
_dangling.add(self)
938938

939-
def _reset_internal_locks(self, is_alive):
940-
# private! Called by _after_fork() to reset our internal locks as
941-
# they may be in an invalid state leading to a deadlock or crash.
939+
def _after_fork(self, new_ident=None):
940+
# Private! Called by threading._after_fork().
942941
self._started._at_fork_reinit()
943-
if is_alive:
942+
if new_ident is not None:
943+
# This thread is alive.
944+
self._ident = new_ident
945+
if self._handle is not None:
946+
self._handle.after_fork_alive()
947+
assert self._handle.ident == new_ident
944948
# bpo-42350: If the fork happens when the thread is already stopped
945949
# (ex: after threading._shutdown() has been called), _tstate_lock
946950
# is None. Do nothing in this case.
@@ -950,11 +954,14 @@ def _reset_internal_locks(self, is_alive):
950954
if self._join_lock is not None:
951955
self._join_lock._at_fork_reinit()
952956
else:
953-
# The thread isn't alive after fork: it doesn't have a tstate
957+
# This thread isn't alive after fork: it doesn't have a tstate
954958
# anymore.
955959
self._is_stopped = True
956960
self._tstate_lock = None
957961
self._join_lock = None
962+
if self._handle is not None:
963+
self._handle.after_fork_dead()
964+
self._handle = None
958965

959966
def __repr__(self):
960967
assert self._initialized, "Thread.__init__() was not called"
@@ -1707,15 +1714,13 @@ def _after_fork():
17071714
# Any lock/condition variable may be currently locked or in an
17081715
# invalid state, so we reinitialize them.
17091716
if thread is current:
1710-
# There is only one active thread. We reset the ident to
1711-
# its new value since it can have changed.
1712-
thread._reset_internal_locks(True)
1717+
# This is the one and only active thread.
17131718
ident = get_ident()
1714-
thread._ident = ident
1719+
thread._after_fork(new_ident=ident)
17151720
new_active[ident] = thread
17161721
else:
17171722
# All the others are already stopped.
1718-
thread._reset_internal_locks(False)
1723+
thread._after_fork()
17191724
thread._stop()
17201725

17211726
_limbo.clear()

Modules/_threadmodule.c

+18
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,22 @@ ThreadHandle_get_ident(ThreadHandleObject *self, void *ignored)
9090
return PyLong_FromUnsignedLongLong(self->ident);
9191
}
9292

93+
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+
93109
static PyObject *
94110
ThreadHandle_detach(ThreadHandleObject *self, void* ignored)
95111
{
@@ -141,6 +157,8 @@ static PyGetSetDef ThreadHandle_getsetlist[] = {
141157

142158
static PyMethodDef ThreadHandle_methods[] =
143159
{
160+
{"after_fork_alive", (PyCFunction)ThreadHandle_after_fork_alive, METH_NOARGS},
161+
{"after_fork_dead", (PyCFunction)ThreadHandle_after_fork_dead, METH_NOARGS},
144162
{"detach", (PyCFunction)ThreadHandle_detach, METH_NOARGS},
145163
{"join", (PyCFunction)ThreadHandle_join, METH_NOARGS},
146164
{0, 0}

Python/thread_nt.h

+4
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,10 @@ PyThread_detach_thread(Py_uintptr_t handle) {
240240
return (CloseHandle(hThread) == 0);
241241
}
242242

243+
void
244+
PyThread_update_thread_after_fork(unsigned long long* ident, Py_uintptr_t* handle) {
245+
}
246+
243247
/*
244248
* Return the thread Id instead of a handle. The Id is said to uniquely identify the
245249
* thread in the system

Python/thread_pthread.h

+10
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,16 @@ PyThread_detach_thread(Py_uintptr_t th) {
337337
return pthread_detach((pthread_t) th);
338338
}
339339

340+
void
341+
PyThread_update_thread_after_fork(unsigned long long* ident, Py_uintptr_t* handle) {
342+
// The thread id might have been updated in the forked child
343+
pthread_t th = pthread_self();
344+
*ident = (unsigned long long) th;
345+
*handle = (Py_uintptr_t) th;
346+
assert(th == (pthread_t) *ident);
347+
assert(th == (pthread_t) *handle);
348+
}
349+
340350
/* XXX This implementation is considered (to quote Tim Peters) "inherently
341351
hosed" because:
342352
- It does not guarantee the promise that a non-zero integer is returned.

0 commit comments

Comments
 (0)