Initial commit

Author: YvesDup

Lib/test/test_thread.py

@@ -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"):

Lib/test/test_threading.py

@@ -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
@@ -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):

Lib/threading.py

@@ -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
@@ -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
@@ -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"
@@ -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:
@@ -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.
@@ -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]
@@ -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.
@@ -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")
@@ -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):
@@ -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
 
@@ -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:
@@ -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:
@@ -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")
 

Modules/_threadmodule.c

@@ -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.
@@ -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()`
@@ -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;
@@ -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;
     }
 
@@ -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);
     }
@@ -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);
@@ -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;
@@ -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))
 {
@@ -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}
 };
@@ -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;
@@ -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;
 }
 
@@ -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;
 }