Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Lib/test/test_thread.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,12 @@ def test_start_duplicate_handle(self):
lock = thread.allocate_lock()
lock.acquire()

handle = thread._ThreadHandle()
def func():
# Simulate the thread bootstrap process.
handle.set_bootstraped()
lock.acquire()

handle = thread._ThreadHandle()
with threading_helper.wait_threads_exit():
thread.start_joinable_thread(func, handle=handle)
with self.assertRaisesRegex(RuntimeError, "thread already started"):
Expand Down
22 changes: 21 additions & 1 deletion Lib/test/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def f(mutex):
# Issue gh-106236:
with self.assertRaises(RuntimeError):
dummy_thread.join()
dummy_thread._started.clear()
dummy_thread._os_thread_handle._set_done()
with self.assertRaises(RuntimeError):
dummy_thread.is_alive()
# Busy wait for the following condition: after the thread dies, the
Expand Down Expand Up @@ -1458,6 +1458,26 @@ def run_in_bg():
self.assertEqual(err, b"")
self.assertEqual(out.strip(), b"Exiting...")

def test_error_bootstrap(self):
# gh-140746: Test that Thread.start() doesn't hang indefinitely if
# the new thread fails (MemoryError in the referenced issue)
# during its initialization

def nothing():
pass

def _set_ident_error():
1/0

with support.catch_unraisable_exception(), self.assertRaises(RuntimeError):
thread = threading.Thread(target=nothing)
thread._set_ident = _set_ident_error
thread.start()
thread.join()
self.assertFalse(thread.is_alive())
self.assertFalse(thread in threading._limbo)
self.assertFalse(thread in threading._active)

class ThreadJoinOnShutdown(BaseTestCase):

def _run_and_join(self, script):
Expand Down
28 changes: 10 additions & 18 deletions Lib/threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,6 @@ class is implemented.
if _HAVE_THREAD_NATIVE_ID:
self._native_id = None
self._os_thread_handle = _ThreadHandle()
self._started = Event()
self._initialized = True
# Copy of sys.stderr used by self._invoke_excepthook()
self._stderr = _sys.stderr
Expand All @@ -940,7 +939,6 @@ class is implemented.

def _after_fork(self, new_ident=None):
# Private! Called by threading._after_fork().
self._started._at_fork_reinit()
if new_ident is not None:
# This thread is alive.
self._ident = new_ident
Expand All @@ -955,7 +953,7 @@ def _after_fork(self, new_ident=None):
def __repr__(self):
assert self._initialized, "Thread.__init__() was not called"
status = "initial"
if self._started.is_set():
if self._os_thread_handle.is_bootstraped():
status = "started"
if self._os_thread_handle.is_done():
status = "stopped"
Expand All @@ -978,7 +976,7 @@ def start(self):
if not self._initialized:
raise RuntimeError("thread.__init__() not called")

if self._started.is_set():
if self._os_thread_handle.is_bootstraped():
raise RuntimeError("threads can only be started once")

with _active_limbo_lock:
Expand All @@ -999,9 +997,9 @@ def start(self):
daemon=self.daemon)
except Exception:
with _active_limbo_lock:
del _limbo[self]
_limbo.pop(self, None)
_active.pop(self._ident, None)
raise
self._started.wait() # Will set ident and native_id

def run(self):
"""Method representing the thread's activity.
Expand Down Expand Up @@ -1061,7 +1059,7 @@ def _bootstrap_inner(self):
if _HAVE_THREAD_NATIVE_ID:
self._set_native_id()
self._set_os_name()
self._started.set()
self._os_thread_handle.set_bootstraped()
with _active_limbo_lock:
_active[self._ident] = self
del _limbo[self]
Expand All @@ -1081,11 +1079,7 @@ def _bootstrap_inner(self):
def _delete(self):
"Remove current thread from the dict of currently running threads."
with _active_limbo_lock:
del _active[get_ident()]
# There must not be any python code between the previous line
# and after the lock is released. Otherwise a tracing function
# could try to acquire the lock again in the same thread, (in
# current_thread()), and would block.
_active.pop(self._ident, None)

def join(self, timeout=None):
"""Wait until the thread terminates.
Expand Down Expand Up @@ -1113,7 +1107,7 @@ def join(self, timeout=None):
"""
if not self._initialized:
raise RuntimeError("Thread.__init__() not called")
if not self._started.is_set():
if not self._os_thread_handle.is_done() and not self._os_thread_handle.is_bootstraped():
raise RuntimeError("cannot join thread before it is started")
if self is current_thread():
raise RuntimeError("cannot join current thread")
Expand Down Expand Up @@ -1176,7 +1170,7 @@ def is_alive(self):

"""
assert self._initialized, "Thread.__init__() not called"
return self._started.is_set() and not self._os_thread_handle.is_done()
return self._os_thread_handle.is_bootstraped() and not self._os_thread_handle.is_done()

@property
def daemon(self):
Expand All @@ -1199,7 +1193,7 @@ def daemon(self, daemonic):
raise RuntimeError("Thread.__init__() not called")
if daemonic and not _daemon_threads_allowed():
raise RuntimeError('daemon threads are disabled in this interpreter')
if self._started.is_set():
if self._os_thread_handle.is_bootstraped() or self._os_thread_handle.is_done():
raise RuntimeError("cannot set daemon status of active thread")
self._daemonic = daemonic

Expand Down Expand Up @@ -1385,7 +1379,6 @@ class _MainThread(Thread):

def __init__(self):
Thread.__init__(self, name="MainThread", daemon=False)
self._started.set()
self._ident = _get_main_thread_ident()
self._os_thread_handle = _make_thread_handle(self._ident)
if _HAVE_THREAD_NATIVE_ID:
Expand Down Expand Up @@ -1433,7 +1426,6 @@ class _DummyThread(Thread):
def __init__(self):
Thread.__init__(self, name=_newname("Dummy-%d"),
daemon=_daemon_threads_allowed())
self._started.set()
self._set_ident()
self._os_thread_handle = _make_thread_handle(self._ident)
if _HAVE_THREAD_NATIVE_ID:
Expand All @@ -1443,7 +1435,7 @@ def __init__(self):
_DeleteDummyThreadOnDel(self)

def is_alive(self):
if not self._os_thread_handle.is_done() and self._started.is_set():
if self._os_thread_handle.is_bootstraped() and not self._os_thread_handle.is_done():
return True
raise RuntimeError("thread is not alive")

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix :func:`threading.Thread.start` that can hang indefinitely in case of heap memory exhaustion
during initialization of the new thread.
83 changes: 78 additions & 5 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ typedef enum {
THREAD_HANDLE_NOT_STARTED = 1,
THREAD_HANDLE_STARTING = 2,
THREAD_HANDLE_RUNNING = 3,
THREAD_HANDLE_DONE = 4,
THREAD_HANDLE_FAILED = 4,
THREAD_HANDLE_DONE = 5,
} ThreadHandleState;

// A handle to wait for thread completion.
Expand Down Expand Up @@ -139,6 +140,7 @@ typedef struct {

PyMutex mutex;

PyEvent thread_is_bootstraped;
// Set immediately before `thread_run` returns to indicate that the OS
// thread is about to exit. This is used to avoid false positives when
// detecting self-join attempts. See the comment in `ThreadHandle_join()`
Expand Down Expand Up @@ -231,6 +233,7 @@ ThreadHandle_new(void)
self->os_handle = 0;
self->has_os_handle = 0;
self->thread_is_exiting = (PyEvent){0};
self->thread_is_bootstraped = (PyEvent){0};
self->mutex = (PyMutex){_Py_UNLOCKED};
self->once = (_PyOnceFlag){0};
self->state = THREAD_HANDLE_NOT_STARTED;
Expand Down Expand Up @@ -286,7 +289,8 @@ ThreadHandle_decref(ThreadHandle *self)
// 1. This is the destructor; nothing else holds a reference.
// 2. The refcount going to zero is a "synchronizes-with" event; all
// changes from other threads are visible.
if (self->state == THREAD_HANDLE_RUNNING && !detach_thread(self)) {
if ((self->state == THREAD_HANDLE_RUNNING || self->state == THREAD_HANDLE_FAILED)
&& !detach_thread(self)) {
self->state = THREAD_HANDLE_DONE;
}

Expand Down Expand Up @@ -322,6 +326,7 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state)
handle->once = (_PyOnceFlag){_Py_ONCE_INITIALIZED};
handle->mutex = (PyMutex){_Py_UNLOCKED};
_PyEvent_Notify(&handle->thread_is_exiting);
_PyEvent_Notify(&handle->thread_is_bootstraped);
llist_remove(node);
remove_from_shutdown_handles(handle);
}
Expand Down Expand Up @@ -393,6 +398,9 @@ thread_run(void *boot_raw)
PyErr_FormatUnraisable(
"Exception ignored in thread started by %R", boot->func);
}
// Notify that the bootstraped is done and failed (e.g. Memory error).
set_thread_handle_state(handle, THREAD_HANDLE_FAILED);
_PyEvent_Notify(&handle->thread_is_bootstraped);
}
else {
Py_DECREF(res);
Expand Down Expand Up @@ -502,7 +510,10 @@ static int
join_thread(void *arg)
{
ThreadHandle *handle = (ThreadHandle*)arg;
assert(get_thread_handle_state(handle) == THREAD_HANDLE_RUNNING);
assert(
get_thread_handle_state(handle) == THREAD_HANDLE_RUNNING ||
get_thread_handle_state(handle) == THREAD_HANDLE_FAILED
);
PyThread_handle_t os_handle;
if (ThreadHandle_get_os_handle(handle, &os_handle)) {
int err = 0;
Expand Down Expand Up @@ -707,6 +718,46 @@ PyThreadHandleObject_join(PyObject *op, PyObject *args)
Py_RETURN_NONE;
}

static PyObject *
PyThreadHandleObject_is_bootstraped(PyObject *op, PyObject *Py_UNUSED(dummy))
{
PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
if (_PyEvent_IsSet(&self->handle->thread_is_bootstraped)) {
Py_RETURN_TRUE;
}
else {
Py_RETURN_FALSE;
}
}

static PyObject *
PyThreadHandleObject_wait_bootstraped(PyObject *op, PyObject *Py_UNUSED(dummy))
{
PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
PyEvent_Wait(&self->handle->thread_is_bootstraped);
Py_RETURN_NONE;
}

static PyObject *
PyThreadHandleObject_set_bootstraped(PyObject *op, PyObject *Py_UNUSED(dummy))
{
PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
_PyEvent_Notify(&self->handle->thread_is_bootstraped);
Py_RETURN_NONE;
}

static PyObject *
PyThreadHandleObject_is_failed(PyObject *op, PyObject *Py_UNUSED(dummy))
{
PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
if (get_thread_handle_state(self->handle) == THREAD_HANDLE_FAILED) {
Py_RETURN_TRUE;
}
else {
Py_RETURN_FALSE;
}
}

static PyObject *
PyThreadHandleObject_is_done(PyObject *op, PyObject *Py_UNUSED(dummy))
{
Expand Down Expand Up @@ -740,6 +791,10 @@ static PyGetSetDef ThreadHandle_getsetlist[] = {
static PyMethodDef ThreadHandle_methods[] = {
{"join", PyThreadHandleObject_join, METH_VARARGS, NULL},
{"_set_done", PyThreadHandleObject_set_done, METH_NOARGS, NULL},
{"wait_bootstraped", PyThreadHandleObject_wait_bootstraped, METH_NOARGS, NULL},
{"set_bootstraped", PyThreadHandleObject_set_bootstraped, METH_NOARGS, NULL},
{"is_bootstraped", PyThreadHandleObject_is_bootstraped, METH_NOARGS, NULL},
{"is_failed", PyThreadHandleObject_is_failed, METH_NOARGS, NULL},
{"is_done", PyThreadHandleObject_is_done, METH_NOARGS, NULL},
{0, 0}
};
Expand Down Expand Up @@ -2025,8 +2080,8 @@ thread_PyThread_start_joinable_thread(PyObject *module, PyObject *fargs,
hobj) < 0) {
return NULL;
}

if (hobj == Py_None) {
int no_handle_arg = (hobj == Py_None);
if (no_handle_arg) {
hobj = (PyObject *)PyThreadHandleObject_new(state->thread_handle_type);
if (hobj == NULL) {
return NULL;
Expand All @@ -2047,6 +2102,23 @@ thread_PyThread_start_joinable_thread(PyObject *module, PyObject *fargs,
Py_DECREF(hobj);
return NULL;
}

// gh-140746: catch error before thread really start
PyThreadHandleObject *thread_handle = PyThreadHandleObject_CAST(hobj);
if (no_handle_arg) {
// If the handle is created by this function, we can be sure that
// the thread is not started before this point. Here, we simulate
// the thread bootstrap process.
_PyEvent_Notify(&thread_handle->handle->thread_is_bootstraped);
}
PyEvent_Wait(&thread_handle->handle->thread_is_bootstraped);

if (get_thread_handle_state(thread_handle->handle) == THREAD_HANDLE_FAILED) {
PyErr_SetString(ThreadError, "call to/in _bootstrap/_bootstrap_inner failed");
Py_DECREF(hobj);
return NULL;
}

return (PyObject *) hobj;
}

Expand Down Expand Up @@ -2467,6 +2539,7 @@ thread__make_thread_handle(PyObject *module, PyObject *identobj)
hobj->handle->ident = ident;
hobj->handle->state = THREAD_HANDLE_RUNNING;
PyMutex_Unlock(&hobj->handle->mutex);
_PyEvent_Notify(&hobj->handle->thread_is_bootstraped);
return (PyObject*) hobj;
}

Expand Down
Loading