Remove detach() method

python · gpshead · Mar 1, 2024 · Feb 8, 2024 · Feb 9, 2024 · Feb 9, 2024
commit cd0372c8d95a350211d71ed0162700b99bc547f7
diff --git a/Lib/test/test_thread.py b/Lib/test/test_thread.py
@@ -233,69 +233,6 @@ def task():
         with self.assertRaisesRegex(RuntimeError, "Cannot join current thread"):
             raise errors[0]
 
-    def test_detach_from_self(self):
-        errors = []
-        handles = []
-        start_joinable_thread_returned = thread.allocate_lock()
-        start_joinable_thread_returned.acquire()
-        thread_detached = thread.allocate_lock()
-        thread_detached.acquire()
-
-        def task():
-            start_joinable_thread_returned.acquire()
-            try:
-                handles[0].detach()
-            except Exception as e:
-                errors.append(e)
-            finally:
-                thread_detached.release()
-
-        with threading_helper.wait_threads_exit():
-            handle = thread.start_joinable_thread(task)
-            handles.append(handle)
-            start_joinable_thread_returned.release()
-            thread_detached.acquire()
-            with self.assertRaisesRegex(ValueError, "detached and thus cannot be joined"):
-                handle.join()
-
-        assert len(errors) == 0
-
-    def test_detach_then_join(self):
-        lock = thread.allocate_lock()
-        lock.acquire()
-
-        def task():
-            lock.acquire()
-
-        with threading_helper.wait_threads_exit():
-            handle = thread.start_joinable_thread(task)
-            # detach() returns even though the thread is blocked on lock
-            handle.detach()
-            # join() then cannot be called anymore
-            with self.assertRaisesRegex(ValueError, "detached and thus cannot be joined"):
-                handle.join()
-            lock.release()
-
-    def test_join_then_detach(self):
-        def task():
-            pass
-
-        with threading_helper.wait_threads_exit():
-            handle = thread.start_joinable_thread(task)
-            handle.join()
-            with self.assertRaisesRegex(ValueError, "joined and thus cannot be detached"):
-                handle.detach()
-
-    def test_detach_then_detach(self):
-        def task():
-            pass
-
-        with threading_helper.wait_threads_exit():
-            handle = thread.start_joinable_thread(task)
-            handle.detach()
-            # Subsequent calls to detach() should succeed
-            handle.detach()
-
     def test_join_then_self_join(self):
         # make sure we can't deadlock in the following scenario with
         # threads t0 and t1:

diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
@@ -51,12 +51,14 @@ typedef enum {
     THREAD_HANDLE_INVALID,
 } ThreadHandleState;
 
-// A handle to join or detach an OS thread.
+// A handle around an OS thread.
 //
-// Joining or detaching the handle is idempotent; the underlying OS thread is
-// joined or detached only once. Concurrent operations block until it is their
-// turn to execute or an operation completes successfully. Once an operation
-// has completed successfully all future operations complete immediately.
+// The OS thread is either joined or detached after the handle is destroyed.
+//
+// Joining the handle is idempotent; the underlying OS thread is joined or
+// detached only once. Concurrent join operations are serialized until it is
+// their turn to execute or an earlier operation completes successfully. Once a
+// join has completed successfully all future joins complete immediately.
 typedef struct {
     PyObject_HEAD
     struct llist_node node;  // linked list node (see _pythread_runtime_state)
@@ -74,7 +76,7 @@ typedef struct {
     // thread is about to exit.
     _PyEventRc *thread_is_exiting;
 
-    // Serializes calls to `join` and `detach`.
+    // Serializes calls to `join`.
     _PyOnceFlag once;
 } ThreadHandleObject;
 
@@ -169,8 +171,8 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state)
             continue;
         }
 
-        // Disallow calls to detach() and join() as they could crash. We are
-        // the only thread; its safe to do this without an atomic.
+        // Disallow calls to join() as they could crash. We are the only
+        // thread; it's safe to set this without an atomic.
         hobj->state = THREAD_HANDLE_INVALID;
         llist_remove(node);
     }
@@ -200,34 +202,6 @@ check_handle_valid(ThreadHandleObject *handle)
     return true;
 }
 
-static PyObject *
-ThreadHandle_detach(ThreadHandleObject *self, void* ignored)
-{
-    if (!check_handle_valid(self)) {
-        return NULL;
-    }
-
-    if (_PyOnceFlag_CallOnce(&self->once, (_Py_once_fn_t *)detach_thread,
-                             self) == -1) {
-        return NULL;
-    }
-
-    switch (get_thread_handle_state(self)) {
-        case THREAD_HANDLE_DETACHED: {
-            Py_RETURN_NONE;
-        }
-        case THREAD_HANDLE_JOINED: {
-            PyErr_SetString(
-                PyExc_ValueError,
-                "the thread has been joined and thus cannot be detached");
-            return NULL;
-        }
-        default: {
-            Py_UNREACHABLE();
-        }
-    }
-}
-
 static int
 join_thread(ThreadHandleObject *handle)
 {
@@ -255,7 +229,7 @@ ThreadHandle_join(ThreadHandleObject *self, void* ignored)
     // We want to perform this check outside of the `_PyOnceFlag` to prevent
     // deadlock in the scenario where another thread joins us and we then
     // attempt to join ourselves. However, it's not safe to check thread
-    // identity once the handle's os thread has finished. We may end up with
+    // identity once the handle's os thread has finished. We may end up reusing
     // the identity stored in the handle and erroneously think we are
     // attempting to join ourselves.
     //
@@ -273,21 +247,8 @@ ThreadHandle_join(ThreadHandleObject *self, void* ignored)
                              self) == -1) {
         return NULL;
     }
-
-    switch (get_thread_handle_state(self)) {
-        case THREAD_HANDLE_DETACHED: {
-            PyErr_SetString(
-                PyExc_ValueError,
-                "the thread is detached and thus cannot be joined");
-            return NULL;
-        }
-        case THREAD_HANDLE_JOINED: {
-            Py_RETURN_NONE;
-        }
-        default: {
-            Py_UNREACHABLE();
-        }
-    }
+    assert(get_thread_handle_state(self) == THREAD_HANDLE_JOINED);
+    Py_RETURN_NONE;
 }
 
 static PyGetSetDef ThreadHandle_getsetlist[] = {
@@ -297,7 +258,6 @@ static PyGetSetDef ThreadHandle_getsetlist[] = {
 
 static PyMethodDef ThreadHandle_methods[] =
 {
-    {"detach", (PyCFunction)ThreadHandle_detach, METH_NOARGS},
     {"join", (PyCFunction)ThreadHandle_join, METH_NOARGS},
     {0, 0}
 };