gh-109746: Make _thread.start_new_thread delete state of new thread on its startup...

If Python fails to start newly created thread
due to failure of underlying PyThread_start_new_thread() call,
its state should be removed from interpreter' thread states list
to avoid its double cleanup.

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
index 3ca5f5ce1b7068036c3222a92c84f609cd5c0381..fb6d268e5869f4a04c2b2e1c91f61c2d4aac3f88 100644 (file)
--- a/Lib/test/test_threading.py
+++ b/Lib/test/test_threading.py
@@ -1171,6 +1171,40 @@ class ThreadTests(BaseTestCase):
         self.assertEqual(out.strip(), b"OK")
         self.assertIn(b"can't create new thread at interpreter shutdown", err)
 
+    def test_start_new_thread_failed(self):
+        # gh-109746: if Python fails to start newly created thread
+        # due to failure of underlying PyThread_start_new_thread() call,
+        # its state should be removed from interpreter' thread states list
+        # to avoid its double cleanup
+        try:
+            from resource import setrlimit, RLIMIT_NPROC
+        except ImportError as err:
+            self.skipTest(err)  # RLIMIT_NPROC is specific to Linux and BSD
+        code = """if 1:
+            import resource
+            import _thread
+
+            def f():
+                print("shouldn't be printed")
+
+            limits = resource.getrlimit(resource.RLIMIT_NPROC)
+            [_, hard] = limits
+            resource.setrlimit(resource.RLIMIT_NPROC, (0, hard))
+
+            try:
+                _thread.start_new_thread(f, ())
+            except RuntimeError:
+                print('ok')
+            else:
+                print('skip')
+        """
+        _, out, err = assert_python_ok("-u", "-c", code)
+        out = out.strip()
+        if out == b'skip':
+            self.skipTest('RLIMIT_NPROC had no effect; probably superuser')
+        self.assertEqual(out, b'ok')
+        self.assertEqual(err, b'')
+
     @cpython_only
     def test_finalize_daemon_thread_hang(self):
         if support.check_sanitizer(thread=True, memory=True):

diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2023-09-22-21-01-56.gh-issue-109746.32MHt9.rst b/Misc/NEWS.d/next/Core_and_Builtins/2023-09-22-21-01-56.gh-issue-109746.32MHt9.rst
new file mode 100644 (file)
index 0000000..2d350c3
--- /dev/null
+++ b/Misc/NEWS.d/next/Core_and_Builtins/2023-09-22-21-01-56.gh-issue-109746.32MHt9.rst
@@ -0,0 +1 @@
+If :func:`!_thread.start_new_thread` fails to start a new thread, it deletes its state from interpreter and thus avoids its repeated cleanup on finalization.

diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
index d4408aa9e42d9dee6039b83f6f1524cbc450feb2..f2a420ac1c589d29805ae15395d9ead27e8b7667 100644 (file)
--- a/Modules/_threadmodule.c
+++ b/Modules/_threadmodule.c
@@ -421,6 +421,7 @@ ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args,
     PyThread_handle_t os_handle;
     if (PyThread_start_joinable_thread(thread_run, boot, &ident, &os_handle)) {
         PyThreadState_Clear(boot->tstate);
+        PyThreadState_Delete(boot->tstate);
         thread_bootstate_free(boot, 1);
         PyErr_SetString(ThreadError, "can't start new thread");
         goto start_failed;

diff --git a/Python/pystate.c b/Python/pystate.c
index 975eb6d4fbd0f2667d9b41182ae3617823fe9074..3ceae229f75cd09e775fafbf56ba3a9eecc1a4d4 100644 (file)
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -1779,7 +1779,9 @@ tstate_delete_common(PyThreadState *tstate, int release_gil)
     if (tstate->_status.bound_gilstate) {
         unbind_gilstate_tstate(tstate);
     }
-    unbind_tstate(tstate);
+    if (tstate->_status.bound) {
+        unbind_tstate(tstate);
+    }
 
     // XXX Move to PyThreadState_Clear()?
     clear_datastack(tstate);