From c0662373bf79968d3a10f900a841774c47ca83ed Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 16:11:49 -0600 Subject: [PATCH 01/11] Add struct module_thread. --- Modules/_threadmodule.c | 76 ++++++++++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 17 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 5d753b4a0ebc5e..6ab0455dddaccc 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -22,6 +22,53 @@ static struct PyModuleDef thread_module; +/* threads owned by the module */ + +struct module_thread { + PyThreadState *tstate; +}; + +static struct module_thread * +new_module_thread(PyInterpreterState *interp) +{ + PyThreadState *tstate = _PyThreadState_New(interp); + if (tstate == NULL) { + if (!PyErr_Occurred()) { + PyErr_NoMemory(); + } + return NULL; + } + + struct module_thread *mt = PyMem_RawMalloc(sizeof(struct module_thread)); + if (mt == NULL) { + PyThreadState_Clear(tstate); + PyThreadState_Delete(tstate); + if (!PyErr_Occurred()) { + PyErr_NoMemory(); + } + return NULL; + } + + *mt = (struct module_thread){ + .tstate = tstate, + }; + return mt; +} + +static void +delete_module_thread(struct module_thread *mt) +{ + if (mt->tstate != NULL) { + PyThreadState_Clear(mt->tstate); + PyThreadState_Delete(mt->tstate); + mt->tstate = NULL; + } + PyMem_RawFree(mt); +} + + +/* module state */ + typedef struct { PyTypeObject *excepthook_type; PyTypeObject *lock_type; @@ -1048,12 +1095,10 @@ _localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref) /* Module functions */ struct bootstate { - PyInterpreterState *interp; + struct module_thread *module_thread; PyObject *func; PyObject *args; PyObject *kwargs; - PyThreadState *tstate; - _PyRuntimeState *runtime; }; @@ -1071,9 +1116,9 @@ static void thread_run(void *boot_raw) { struct bootstate *boot = (struct bootstate *) boot_raw; - PyThreadState *tstate; + struct module_thread *mt = boot->module_thread; + PyThreadState *tstate = mt->tstate; - tstate = boot->tstate; _PyThreadState_Bind(tstate); PyEval_AcquireThread(tstate); tstate->interp->threads.count++; @@ -1095,6 +1140,8 @@ thread_run(void *boot_raw) tstate->interp->threads.count--; PyThreadState_Clear(tstate); _PyThreadState_DeleteCurrent(tstate); + mt->tstate = NULL; + delete_module_thread(mt); // bpo-44434: Don't call explicitly PyThread_exit_thread(). On Linux with // the glibc, pthread_exit() can abort the whole process if dlopen() fails @@ -1122,7 +1169,6 @@ and False otherwise.\n"); static PyObject * thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) { - _PyRuntimeState *runtime = &_PyRuntime; PyObject *func, *args, *kwargs = NULL; if (!PyArg_UnpackTuple(fargs, "start_new_thread", 2, 3, @@ -1156,20 +1202,16 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) return NULL; } + struct module_thread *mt = new_module_thread(interp); + if (mt == NULL) { + return NULL; + } + struct bootstate *boot = PyMem_NEW(struct bootstate, 1); if (boot == NULL) { return PyErr_NoMemory(); } - boot->interp = _PyInterpreterState_GET(); - boot->tstate = _PyThreadState_New(boot->interp); - if (boot->tstate == NULL) { - PyMem_Free(boot); - if (!PyErr_Occurred()) { - return PyErr_NoMemory(); - } - return NULL; - } - boot->runtime = runtime; + boot->module_thread = mt; boot->func = Py_NewRef(func); boot->args = Py_NewRef(args); boot->kwargs = Py_XNewRef(kwargs); @@ -1177,7 +1219,7 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) unsigned long ident = PyThread_start_new_thread(thread_run, (void*) boot); if (ident == PYTHREAD_INVALID_THREAD_ID) { PyErr_SetString(ThreadError, "can't start new thread"); - PyThreadState_Clear(boot->tstate); + delete_module_thread(mt); thread_bootstate_free(boot); return NULL; } From d10a24f17c98152d97a0a36c5a30639b72d2680f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 16:25:21 -0600 Subject: [PATCH 02/11] Add module_thread.daemonic. --- Lib/test/test_threading.py | 2 +- Lib/threading.py | 2 +- Modules/_threadmodule.c | 25 ++++++++++++++++--------- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 97165264b34bbe..222e096f57e3e0 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -345,7 +345,7 @@ def run(self): def test_limbo_cleanup(self): # Issue 7481: Failure to start thread should cleanup the limbo map. - def fail_new_thread(*args): + def fail_new_thread(*args, **kwargs): raise threading.ThreadError() _start_new_thread = threading._start_new_thread threading._start_new_thread = fail_new_thread diff --git a/Lib/threading.py b/Lib/threading.py index df273870fa4273..77e2af864b518a 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -968,7 +968,7 @@ def start(self): with _active_limbo_lock: _limbo[self] = self try: - _start_new_thread(self._bootstrap, ()) + _start_new_thread(self._bootstrap, (), daemonic=self._daemonic) except Exception: with _active_limbo_lock: del _limbo[self] diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 6ab0455dddaccc..faa69818c953a2 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -26,10 +26,11 @@ static struct PyModuleDef thread_module; struct module_thread { PyThreadState *tstate; + int daemonic; }; static struct module_thread * -new_module_thread(PyInterpreterState *interp) +new_module_thread(PyInterpreterState *interp, int daemonic) { PyThreadState *tstate = _PyThreadState_New(interp); if (tstate == NULL) { @@ -51,6 +52,7 @@ new_module_thread(PyInterpreterState *interp) *mt = (struct module_thread){ .tstate = tstate, + .daemonic = daemonic, }; return mt; } @@ -1167,13 +1169,18 @@ Return True if daemon threads are allowed in the current interpreter,\n\ and False otherwise.\n"); static PyObject * -thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) +thread_PyThread_start_new_thread(PyObject *self, + PyObject *fargs, PyObject *fkwargs) { + char *kwlist[] = {"", "", "", "daemonic", NULL}; PyObject *func, *args, *kwargs = NULL; - - if (!PyArg_UnpackTuple(fargs, "start_new_thread", 2, 3, - &func, &args, &kwargs)) + int daemonic = 0; + if (!PyArg_ParseTupleAndKeywords(fargs, fkwargs, + "OO|Op:start_new_thread", kwlist, + &func, &args, &kwargs, &daemonic)) + { return NULL; + } if (!PyCallable_Check(func)) { PyErr_SetString(PyExc_TypeError, "first arg must be callable"); @@ -1202,7 +1209,7 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) return NULL; } - struct module_thread *mt = new_module_thread(interp); + struct module_thread *mt = new_module_thread(interp, daemonic); if (mt == NULL) { return NULL; } @@ -1227,7 +1234,7 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs) } PyDoc_STRVAR(start_new_doc, -"start_new_thread(function, args[, kwargs])\n\ +"start_new_thread(function, args[, kwargs], daemonic=0)\n\ (start_new() is an obsolete synonym)\n\ \n\ Start a new thread and return its identifier. The thread will call the\n\ @@ -1607,9 +1614,9 @@ Handle uncaught Thread.run() exception."); static PyMethodDef thread_methods[] = { {"start_new_thread", (PyCFunction)thread_PyThread_start_new_thread, - METH_VARARGS, start_new_doc}, + METH_VARARGS | METH_KEYWORDS, start_new_doc}, {"start_new", (PyCFunction)thread_PyThread_start_new_thread, - METH_VARARGS, start_new_doc}, + METH_VARARGS | METH_KEYWORDS, start_new_doc}, {"daemon_threads_allowed", (PyCFunction)thread_daemon_threads_allowed, METH_NOARGS, daemon_threads_allowed_doc}, {"allocate_lock", thread_PyThread_allocate_lock, From dfcc14f15e86ec17c59e283a4f78f52e22d2102c Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 16:55:11 -0600 Subject: [PATCH 03/11] Add module_thread.status and related helpers. --- Modules/_threadmodule.c | 81 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 72 insertions(+), 9 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index faa69818c953a2..77582a0053aab2 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -27,6 +27,15 @@ static struct PyModuleDef thread_module; struct module_thread { PyThreadState *tstate; int daemonic; + struct { + unsigned int initialized:1; + unsigned int started:1; + unsigned int func_started:1; + unsigned int func_ended:1; + unsigned int tstate_cleared:1; + /* padding to align to 4 bytes */ + unsigned int :27; + } status; }; static struct module_thread * @@ -53,6 +62,9 @@ new_module_thread(PyInterpreterState *interp, int daemonic) *mt = (struct module_thread){ .tstate = tstate, .daemonic = daemonic, + .status = { + .initialized = 1, + }, }; return mt; } @@ -61,6 +73,7 @@ static void delete_module_thread(struct module_thread *mt) { if (mt->tstate != NULL) { + assert(!mt->status.tstate_cleared); PyThreadState_Clear(mt->tstate); PyThreadState_Delete(mt->tstate); mt->tstate = NULL; @@ -68,6 +81,58 @@ delete_module_thread(struct module_thread *mt) PyMem_RawFree(mt); } +static void +bind_module_thread(struct module_thread *mt) +{ + assert(_PyThreadState_GET() == NULL); + assert(mt->status.initialized); + assert(!mt->status.started); + + mt->status.started = 1; + + _PyThreadState_Bind(mt->tstate); +} + +static void +set_module_thread_starting(struct module_thread *mt) +{ + assert(mt->tstate == _PyThreadState_GET()); + assert(mt->status.started); + assert(!mt->status.func_started); + + mt->status.func_started = 1; + + mt->tstate->interp->threads.count++; +} + +static void +set_module_thread_finished(struct module_thread *mt) +{ + assert(mt->tstate == _PyThreadState_GET()); + assert(mt->status.func_started); + assert(!mt->status.func_ended); + + mt->status.func_ended = 1; + + mt->tstate->interp->threads.count--; +} + +static void +release_module_thread_tstate(struct module_thread *mt) +{ + assert(mt->tstate == _PyThreadState_GET()); + assert(mt->status.func_ended); + assert(!mt->status.tstate_cleared); + + PyThreadState *tstate = mt->tstate; + PyThreadState_Clear(tstate); + // This releases the GIL. + _PyThreadState_DeleteCurrent(tstate); + + mt->tstate = NULL; + mt->status.tstate_cleared = 1; +} + /* module state */ @@ -1119,12 +1184,12 @@ thread_run(void *boot_raw) { struct bootstate *boot = (struct bootstate *) boot_raw; struct module_thread *mt = boot->module_thread; - PyThreadState *tstate = mt->tstate; - _PyThreadState_Bind(tstate); - PyEval_AcquireThread(tstate); - tstate->interp->threads.count++; + bind_module_thread(mt); + // Run the Python function with the GIL held. + PyEval_AcquireThread(mt->tstate); + set_module_thread_starting(mt); PyObject *res = PyObject_Call(boot->func, boot->args, boot->kwargs); if (res == NULL) { if (PyErr_ExceptionMatches(PyExc_SystemExit)) @@ -1137,12 +1202,10 @@ thread_run(void *boot_raw) else { Py_DECREF(res); } - + set_module_thread_finished(mt); thread_bootstate_free(boot); - tstate->interp->threads.count--; - PyThreadState_Clear(tstate); - _PyThreadState_DeleteCurrent(tstate); - mt->tstate = NULL; + + release_module_thread_tstate(mt); delete_module_thread(mt); // bpo-44434: Don't call explicitly PyThread_exit_thread(). On Linux with From 6fca20e5faaecf844e43baebb1cd75af1d7a7b98 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 17:37:43 -0600 Subject: [PATCH 04/11] Add struct module_threads. --- Lib/threading.py | 6 ++ Modules/_threadmodule.c | 164 +++++++++++++++++++++++++++++++++++----- 2 files changed, 152 insertions(+), 18 deletions(-) diff --git a/Lib/threading.py b/Lib/threading.py index 77e2af864b518a..1d5d9dd98d5551 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -49,6 +49,10 @@ except AttributeError: _CRLock = None TIMEOUT_MAX = _thread.TIMEOUT_MAX +try: + _internal_after_fork = _thread._after_fork +except AttributeError: + _internal_after_fork = None del _thread @@ -1677,4 +1681,6 @@ def _after_fork(): if hasattr(_os, "register_at_fork"): + if _internal_after_fork is not None: + _os.register_at_fork(after_in_child=_internal_after_fork) _os.register_at_fork(after_in_child=_after_fork) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 77582a0053aab2..31e463b5fac362 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -22,7 +22,7 @@ static struct PyModuleDef thread_module; -/* threads owned by the module */ +/* state for threads owned by the module */ struct module_thread { PyThreadState *tstate; @@ -81,62 +81,163 @@ delete_module_thread(struct module_thread *mt) PyMem_RawFree(mt); } + +struct module_threads { + PyThread_type_lock mutex; + struct { + long all; + long running; + long non_daemon_running; + long pyfuncs_running; + } counts; +}; + +static int +module_threads_init(struct module_threads *threads) +{ + PyThread_type_lock lock = PyThread_allocate_lock(); + if (lock == NULL) { + PyErr_NoMemory(); + return -1; + } + + *threads = (struct module_threads){ + .mutex = lock, + }; + assert(_PyThreadState_GET()->interp->threads.count == 0); + return 0; +} + +#ifdef HAVE_FORK +static int +module_threads_reinit(struct module_threads *threads) +{ + PyThreadState *tstate = _PyThreadState_GET(); + assert(tstate->thread_id == PyThread_get_thread_ident()); + + if (_PyThread_at_fork_reinit(threads->mutex) < 0) { + PyErr_SetString(ThreadError, "failed to reinitialize lock at fork"); + return -1; + } + + *threads = (struct module_threads){ + .mutex = threads->mutex, + // The counts are all reset to 0. + }; + // XXX assert(_PyThreadState_GET()->interp->threads.count == 0); + return 0; +} +#endif + static void -bind_module_thread(struct module_thread *mt) +module_threads_fini(struct module_threads *threads) +{ + // XXX assert(_PyThreadState_GET()->interp->threads.count == threads->counts.pyfuncs_running); + + // XXX Wait for all module threads to finish running thread_run(). + assert(threads->counts.running == 0); + + PyThread_free_lock(threads->mutex); +} + + +/* high-level helpers for threads owned by the module */ + +static void +bind_module_thread(struct module_threads *threads, + struct module_thread *mt) { assert(_PyThreadState_GET() == NULL); + + PyThread_acquire_lock(threads->mutex, WAIT_LOCK); + assert(mt->status.initialized); assert(!mt->status.started); - mt->status.started = 1; + threads->counts.all++; + threads->counts.running++; + + PyThread_release_lock(threads->mutex); _PyThreadState_Bind(mt->tstate); } static void -set_module_thread_starting(struct module_thread *mt) +set_module_thread_starting(struct module_threads *threads, + struct module_thread *mt) { + PyThread_acquire_lock(threads->mutex, WAIT_LOCK); + assert(mt->tstate == _PyThreadState_GET()); assert(mt->status.started); assert(!mt->status.func_started); - mt->status.func_started = 1; - + threads->counts.pyfuncs_running++; + // XXX Drop interp.threads.count. mt->tstate->interp->threads.count++; + + PyThread_release_lock(threads->mutex); } static void -set_module_thread_finished(struct module_thread *mt) +set_module_thread_finished(struct module_threads *threads, + struct module_thread *mt) { + PyThread_acquire_lock(threads->mutex, WAIT_LOCK); + assert(mt->tstate == _PyThreadState_GET()); assert(mt->status.func_started); assert(!mt->status.func_ended); - mt->status.func_ended = 1; - + threads->counts.pyfuncs_running--; + // XXX Drop interp.threads.count. mt->tstate->interp->threads.count--; + + PyThread_release_lock(threads->mutex); } static void -release_module_thread_tstate(struct module_thread *mt) +release_module_thread_tstate(struct module_threads *threads, + struct module_thread *mt) { + PyThread_acquire_lock(threads->mutex, WAIT_LOCK); + assert(mt->tstate == _PyThreadState_GET()); assert(mt->status.func_ended); assert(!mt->status.tstate_cleared); - PyThreadState *tstate = mt->tstate; + mt->tstate = NULL; + mt->status.tstate_cleared = 1; + + PyThread_release_lock(threads->mutex); + PyThreadState_Clear(tstate); // This releases the GIL. _PyThreadState_DeleteCurrent(tstate); +} - mt->tstate = NULL; - mt->status.tstate_cleared = 1; +static void +finalize_module_thread(struct module_threads *threads, + struct module_thread *mt) +{ + PyThread_acquire_lock(threads->mutex, WAIT_LOCK); + + assert(mt->tstate == _PyThreadState_GET()); + assert(mt->status.func_ended); + assert(mt->status.tstate_cleared); + threads->counts.running--; + + PyThread_release_lock(threads->mutex); + + delete_module_thread(mt); } /* module state */ typedef struct { + struct module_threads threads; + PyTypeObject *excepthook_type; PyTypeObject *lock_type; PyTypeObject *local_type; @@ -1162,6 +1263,7 @@ _localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref) /* Module functions */ struct bootstate { + thread_module_state *module_state; struct module_thread *module_thread; PyObject *func; PyObject *args; @@ -1183,13 +1285,14 @@ static void thread_run(void *boot_raw) { struct bootstate *boot = (struct bootstate *) boot_raw; + struct module_threads *threads = &boot->module_state->threads; struct module_thread *mt = boot->module_thread; - bind_module_thread(mt); + bind_module_thread(threads, mt); // Run the Python function with the GIL held. PyEval_AcquireThread(mt->tstate); - set_module_thread_starting(mt); + set_module_thread_starting(threads, mt); PyObject *res = PyObject_Call(boot->func, boot->args, boot->kwargs); if (res == NULL) { if (PyErr_ExceptionMatches(PyExc_SystemExit)) @@ -1202,11 +1305,11 @@ thread_run(void *boot_raw) else { Py_DECREF(res); } - set_module_thread_finished(mt); + set_module_thread_finished(threads, mt); thread_bootstate_free(boot); - release_module_thread_tstate(mt); - delete_module_thread(mt); + release_module_thread_tstate(threads, mt); + finalize_module_thread(threads, mt); // bpo-44434: Don't call explicitly PyThread_exit_thread(). On Linux with // the glibc, pthread_exit() can abort the whole process if dlopen() fails @@ -1271,6 +1374,7 @@ thread_PyThread_start_new_thread(PyObject *self, "thread is not supported for isolated subinterpreters"); return NULL; } + thread_module_state *state = get_thread_state(self); struct module_thread *mt = new_module_thread(interp, daemonic); if (mt == NULL) { @@ -1281,6 +1385,7 @@ thread_PyThread_start_new_thread(PyObject *self, if (boot == NULL) { return PyErr_NoMemory(); } + boot->module_state = state; boot->module_thread = mt; boot->func = Py_NewRef(func); boot->args = Py_NewRef(args); @@ -1675,6 +1780,18 @@ PyDoc_STRVAR(excepthook_doc, \n\ Handle uncaught Thread.run() exception."); +#ifdef HAVE_FORK +static PyObject * +thread__after_fork(PyObject *module, PyObject *Py_UNUSED(ignored)) +{ + thread_module_state *state = get_thread_state(module); + if (module_threads_reinit(&state->threads) < 0) { + return NULL; + } + Py_RETURN_NONE; +} +#endif + static PyMethodDef thread_methods[] = { {"start_new_thread", (PyCFunction)thread_PyThread_start_new_thread, METH_VARARGS | METH_KEYWORDS, start_new_doc}, @@ -1706,6 +1823,10 @@ static PyMethodDef thread_methods[] = { METH_NOARGS, _set_sentinel_doc}, {"_excepthook", thread_excepthook, METH_O, excepthook_doc}, +#ifdef HAVE_FORK + {"_after_fork", (PyCFunction)thread__after_fork, + METH_NOARGS, NULL}, +#endif {NULL, NULL} /* sentinel */ }; @@ -1721,6 +1842,11 @@ thread_module_exec(PyObject *module) // Initialize the C thread library PyThread_init_thread(); + // Initialize the list of threads owned by this module. + if (module_threads_init(&state->threads) < 0) { + return -1; + } + // Lock state->lock_type = (PyTypeObject *)PyType_FromSpec(&lock_type_spec); if (state->lock_type == NULL) { @@ -1811,6 +1937,8 @@ thread_module_clear(PyObject *module) static void thread_module_free(void *module) { + thread_module_state *state = get_thread_state(module); + module_threads_fini(&state->threads); thread_module_clear((PyObject *)module); } From 921b1ec9a0ae00a8a60328831c3a09847444ec07 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 18:22:50 -0600 Subject: [PATCH 05/11] Free bootstate before running PyObject_Call(). --- Modules/_threadmodule.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 31e463b5fac362..816858c3f09264 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1287,27 +1287,38 @@ thread_run(void *boot_raw) struct bootstate *boot = (struct bootstate *) boot_raw; struct module_threads *threads = &boot->module_state->threads; struct module_thread *mt = boot->module_thread; + PyObject *pyfunc = boot->func; + PyObject *pyargs = boot->args; + PyObject *pykwargs = boot->kwargs; bind_module_thread(threads, mt); - // Run the Python function with the GIL held. PyEval_AcquireThread(mt->tstate); + // We free the boot state before running pyfunc + // since daemon threads can exit before PyObject_Call() returns. + // We can't do much about leaking pyfunc/pyargs/pykwargs though. + PyMem_Free(boot); + + // Run the Python function with the GIL held. set_module_thread_starting(threads, mt); - PyObject *res = PyObject_Call(boot->func, boot->args, boot->kwargs); + PyObject *res = PyObject_Call(pyfunc, pyargs, pykwargs); if (res == NULL) { if (PyErr_ExceptionMatches(PyExc_SystemExit)) /* SystemExit is ignored silently */ PyErr_Clear(); else { - _PyErr_WriteUnraisableMsg("in thread started by", boot->func); + _PyErr_WriteUnraisableMsg("in thread started by", pyfunc); } } else { Py_DECREF(res); } set_module_thread_finished(threads, mt); - thread_bootstate_free(boot); + // Clean up everything we created in thread_PyThread_start_new_thread(). + Py_DECREF(pyfunc); + Py_DECREF(pyargs); + Py_XDECREF(pykwargs); release_module_thread_tstate(threads, mt); finalize_module_thread(threads, mt); From fcee99e7df33f27394d5d654dac2a6ec5be688d7 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 18:28:56 -0600 Subject: [PATCH 06/11] Drop PyInterpreterState.threads.count. --- Include/internal/pycore_interp.h | 2 -- Modules/_threadmodule.c | 16 ++++++---------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index edc076fc04f6c3..98f620c59c6b24 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -57,8 +57,6 @@ struct _is { uint64_t next_unique_id; /* The linked list of threads, newest first. */ PyThreadState *head; - /* Used in Modules/_threadmodule.c. */ - long count; /* Support for runtime thread stack size tuning. A value of 0 means using the platform's default stack size or the size specified by the THREAD_STACK_SIZE macro. */ diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 816858c3f09264..2cda99b05a9aa8 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -104,7 +104,6 @@ module_threads_init(struct module_threads *threads) *threads = (struct module_threads){ .mutex = lock, }; - assert(_PyThreadState_GET()->interp->threads.count == 0); return 0; } @@ -124,7 +123,6 @@ module_threads_reinit(struct module_threads *threads) .mutex = threads->mutex, // The counts are all reset to 0. }; - // XXX assert(_PyThreadState_GET()->interp->threads.count == 0); return 0; } #endif @@ -132,10 +130,12 @@ module_threads_reinit(struct module_threads *threads) static void module_threads_fini(struct module_threads *threads) { - // XXX assert(_PyThreadState_GET()->interp->threads.count == threads->counts.pyfuncs_running); + PyThread_acquire_lock(threads->mutex, WAIT_LOCK); // XXX Wait for all module threads to finish running thread_run(). - assert(threads->counts.running == 0); + // XXX assert(threads->counts.running == 0); + + PyThread_release_lock(threads->mutex); PyThread_free_lock(threads->mutex); } @@ -173,8 +173,6 @@ set_module_thread_starting(struct module_threads *threads, assert(!mt->status.func_started); mt->status.func_started = 1; threads->counts.pyfuncs_running++; - // XXX Drop interp.threads.count. - mt->tstate->interp->threads.count++; PyThread_release_lock(threads->mutex); } @@ -190,8 +188,6 @@ set_module_thread_finished(struct module_threads *threads, assert(!mt->status.func_ended); mt->status.func_ended = 1; threads->counts.pyfuncs_running--; - // XXX Drop interp.threads.count. - mt->tstate->interp->threads.count--; PyThread_release_lock(threads->mutex); } @@ -1519,8 +1515,8 @@ particular thread within a system."); static PyObject * thread__count(PyObject *self, PyObject *Py_UNUSED(ignored)) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - return PyLong_FromLong(interp->threads.count); + thread_module_state *state = get_thread_state(self); + return PyLong_FromLong(state->threads.counts.pyfuncs_running); } PyDoc_STRVAR(_count_doc, From 8da0aeec5b2dce3ea366c38f93a0d1840bee72f6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 18:40:13 -0600 Subject: [PATCH 07/11] Add a TODO. --- Modules/_threadmodule.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 2cda99b05a9aa8..45d99abe0894d3 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -190,6 +190,9 @@ set_module_thread_finished(struct module_threads *threads, threads->counts.pyfuncs_running--; PyThread_release_lock(threads->mutex); + + // Notify other threads that this one is done. + // XXX Do it explicitly here rather than via tstate.on_delete(). } static void From a7016b2694b100a3ecee56eccc9823516af29ea4 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 18:41:37 -0600 Subject: [PATCH 08/11] Drop an unused field. --- Modules/_threadmodule.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 45d99abe0894d3..0780112f204789 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -87,7 +87,6 @@ struct module_threads { struct { long all; long running; - long non_daemon_running; long pyfuncs_running; } counts; }; From a44c03f4f8662a5cd6bd85cb7b62d6d1354ab7a6 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 20:14:11 -0600 Subject: [PATCH 09/11] Fix untrack_module_thread(). --- Modules/_threadmodule.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 0780112f204789..11542e53d3f94d 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -215,19 +215,18 @@ release_module_thread_tstate(struct module_threads *threads, } static void -finalize_module_thread(struct module_threads *threads, +untrack_module_thread(struct module_threads *threads, struct module_thread *mt) { + assert(_PyThreadState_GET() == NULL); + PyThread_acquire_lock(threads->mutex, WAIT_LOCK); - assert(mt->tstate == _PyThreadState_GET()); assert(mt->status.func_ended); assert(mt->status.tstate_cleared); threads->counts.running--; PyThread_release_lock(threads->mutex); - - delete_module_thread(mt); } @@ -1318,7 +1317,8 @@ thread_run(void *boot_raw) Py_DECREF(pyargs); Py_XDECREF(pykwargs); release_module_thread_tstate(threads, mt); - finalize_module_thread(threads, mt); + untrack_module_thread(threads, mt); + delete_module_thread(mt); // bpo-44434: Don't call explicitly PyThread_exit_thread(). On Linux with // the glibc, pthread_exit() can abort the whole process if dlopen() fails From 7cca8fa67728b3aea8cc4255be091fc50b557921 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 20:25:41 -0600 Subject: [PATCH 10/11] Fix warnings. --- Modules/_threadmodule.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 11542e53d3f94d..e8e58ed1381227 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -110,7 +110,9 @@ module_threads_init(struct module_threads *threads) static int module_threads_reinit(struct module_threads *threads) { +#ifndef NDEBUG PyThreadState *tstate = _PyThreadState_GET(); +#endif assert(tstate->thread_id == PyThread_get_thread_ident()); if (_PyThread_at_fork_reinit(threads->mutex) < 0) { @@ -1802,9 +1804,9 @@ thread__after_fork(PyObject *module, PyObject *Py_UNUSED(ignored)) #endif static PyMethodDef thread_methods[] = { - {"start_new_thread", (PyCFunction)thread_PyThread_start_new_thread, + {"start_new_thread", _PyCFunction_CAST(thread_PyThread_start_new_thread), METH_VARARGS | METH_KEYWORDS, start_new_doc}, - {"start_new", (PyCFunction)thread_PyThread_start_new_thread, + {"start_new", _PyCFunction_CAST(thread_PyThread_start_new_thread), METH_VARARGS | METH_KEYWORDS, start_new_doc}, {"daemon_threads_allowed", (PyCFunction)thread_daemon_threads_allowed, METH_NOARGS, daemon_threads_allowed_doc}, From d1e770c559e1a09697277866ae45f6e5ab9d4e00 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 17 May 2023 20:44:34 -0600 Subject: [PATCH 11/11] Fix module_threads_reinit(). --- Modules/_threadmodule.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index e8e58ed1381227..7a71adc73e7997 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -115,13 +115,15 @@ module_threads_reinit(struct module_threads *threads) #endif assert(tstate->thread_id == PyThread_get_thread_ident()); - if (_PyThread_at_fork_reinit(threads->mutex) < 0) { + PyThread_type_lock lock = threads->mutex; + assert(lock != NULL); + if (_PyThread_at_fork_reinit(&lock) < 0) { PyErr_SetString(ThreadError, "failed to reinitialize lock at fork"); return -1; } *threads = (struct module_threads){ - .mutex = threads->mutex, + .mutex = lock, // The counts are all reset to 0. }; return 0;