Fixes of my previous try:

- Add a new type of ThreadHandleState for failing bootstrap (avoid
using THREAD_HANDLE_DONE for two different case, which is incorrect
for join())
- is_bootstraped instead of is_running having the same behavior than
before (True when the thread is `THREAD_HANDLE_DONE` state)
- Renamming stuff

Author: Rémy Voet (ryv)

Lib/test/test_threading.py

@@ -1476,6 +1476,7 @@ def _set_ident_memory_error():
                 mock.patch.object(thread, '_set_ident', _set_ident_memory_error)
             ):
                 thread.start()
+                thread.join()
                 self.assertFalse(thread.is_alive())
                 self.assertFalse(thread in threading._limbo)
                 self.assertFalse(thread in threading._active)

Lib/threading.py

@@ -953,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._os_thread_handle.is_running():
+        if self._os_thread_handle.is_bootstraped():
             status = "started"
         if self._os_thread_handle.is_done():
             status = "stopped"
@@ -976,7 +976,7 @@ def start(self):
         if not self._initialized:
             raise RuntimeError("thread.__init__() not called")
 
-        if self._os_thread_handle.is_running():
+        if self._os_thread_handle.is_bootstraped():
             raise RuntimeError("threads can only be started once")
 
         with _active_limbo_lock:
@@ -999,13 +999,15 @@ def start(self):
             with _active_limbo_lock:
                 del _limbo[self]
             raise
-        self._os_thread_handle.wait_bootstrap_done()
+        self._os_thread_handle.wait_bootstraped()
 
         # It's possible that the _started event never occurs from the new Thread;
         # e.g., it didn't have enough memory to call the initialization part of _bootstrap_inner.
-        if self._os_thread_handle.is_done():
+        if self._os_thread_handle.is_failed():
             with _active_limbo_lock:
                 _limbo.pop(self, None)
+                if self._ident:
+                    _active.pop(self._ident, None)
 
     def run(self):
         """Method representing the thread's activity.
@@ -1065,7 +1067,7 @@ def _bootstrap_inner(self):
             if _HAVE_THREAD_NATIVE_ID:
                 self._set_native_id()
             self._set_os_name()
-            self._os_thread_handle.set_bootstrap_done()
+            self._os_thread_handle.set_bootstraped()
             with _active_limbo_lock:
                 _active[self._ident] = self
                 del _limbo[self]
@@ -1113,7 +1115,7 @@ def join(self, timeout=None):
         """
         if not self._initialized:
             raise RuntimeError("Thread.__init__() not called")
-        if not self._os_thread_handle.is_done() and not self._os_thread_handle.is_running():
+        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 +1178,7 @@ def is_alive(self):
 
         """
         assert self._initialized, "Thread.__init__() not called"
-        return self._os_thread_handle.is_running() 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 +1201,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._os_thread_handle.is_running() or self._os_thread_handle.is_done():
+        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
 
@@ -1441,7 +1443,7 @@ def __init__(self):
         _DeleteDummyThreadOnDel(self)
 
     def is_alive(self):
-        if self._os_thread_handle.is_running() and not self._os_thread_handle.is_done():
+        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.
@@ -288,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;
     }
 
@@ -396,8 +398,8 @@ thread_run(void *boot_raw)
             PyErr_FormatUnraisable(
                 "Exception ignored in thread started by %R", boot->func);
         }
-        // TODO: should we use a new state for failed bootsrap Thread
-        set_thread_handle_state(handle, THREAD_HANDLE_DONE);
+        set_thread_handle_state(handle, THREAD_HANDLE_FAILED);
+        // Notify that the bootstraped is done, in case the Python call didn't manage too (e.g. Memory error)
         _PyEvent_Notify(&handle->thread_is_bootstraped);
     }
     else {
@@ -509,7 +511,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;
@@ -629,7 +634,7 @@ ThreadHandle_set_done(ThreadHandle *self)
 }
 
 static void
-ThreadHandle_set_bootstrap_done(ThreadHandle *self)
+ThreadHandle_set_bootstraped(ThreadHandle *self)
 {
     _PyEvent_Notify(&self->thread_is_bootstraped);
 }
@@ -721,11 +726,10 @@ PyThreadHandleObject_join(PyObject *op, PyObject *args)
 }
 
 static PyObject *
-PyThreadHandleObject_is_running(PyObject *op, PyObject *Py_UNUSED(dummy))
+PyThreadHandleObject_is_bootstraped(PyObject *op, PyObject *Py_UNUSED(dummy))
 {
     PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
-    ThreadHandleState state = get_thread_handle_state(self->handle);
-    if (state == THREAD_HANDLE_RUNNING) {
+    if (_PyEvent_IsSet(&self->handle->thread_is_bootstraped)) {
         Py_RETURN_TRUE;
     }
     else {
@@ -734,21 +738,33 @@ PyThreadHandleObject_is_running(PyObject *op, PyObject *Py_UNUSED(dummy))
 }
 
 static PyObject *
-PyThreadHandleObject_wait_bootstrap_done(PyObject *op, PyObject *Py_UNUSED(dummy))
+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_bootstrap_done(PyObject *op, PyObject *Py_UNUSED(dummy))
+PyThreadHandleObject_set_bootstraped(PyObject *op, PyObject *Py_UNUSED(dummy))
 {
     PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
-    ThreadHandle_set_bootstrap_done(self->handle);
+    ThreadHandle_set_bootstraped(self->handle);
     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))
 {
@@ -782,9 +798,10 @@ static PyGetSetDef ThreadHandle_getsetlist[] = {
 static PyMethodDef ThreadHandle_methods[] = {
     {"join", PyThreadHandleObject_join, METH_VARARGS, NULL},
     {"_set_done", PyThreadHandleObject_set_done, METH_NOARGS, NULL},
-    {"wait_bootstrap_done", PyThreadHandleObject_wait_bootstrap_done, METH_NOARGS, NULL},
-    {"set_bootstrap_done", PyThreadHandleObject_set_bootstrap_done, METH_NOARGS, NULL},
-    {"is_running", PyThreadHandleObject_is_running, 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}
 };
@@ -2511,6 +2528,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;
 }