From 8408cea0cdc0ccd5900acd99a9a51dd9161ae319 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Sun, 5 May 2013 23:47:09 +0200 Subject: [PATCH] Issue #17094: Clear stale thread states after fork(). Note that this is a potentially disruptive change since it may release some system resources which would otherwise remain perpetually alive (e.g. database connections kept in thread-local storage). --- Include/pystate.h | 1 + Lib/test/test_threading.py | 25 ++++++++++++++++++++ Misc/NEWS | 5 ++++ Python/ceval.c | 18 ++++++++------- Python/pystate.c | 47 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 88 insertions(+), 8 deletions(-) diff --git a/Include/pystate.h b/Include/pystate.h index b29ce2a482f..a8fcc73bcb3 100644 --- a/Include/pystate.h +++ b/Include/pystate.h @@ -139,6 +139,7 @@ PyAPI_FUNC(PyThreadState *) _PyThreadState_Prealloc(PyInterpreterState *); PyAPI_FUNC(void) _PyThreadState_Init(PyThreadState *); PyAPI_FUNC(void) PyThreadState_Clear(PyThreadState *); PyAPI_FUNC(void) PyThreadState_Delete(PyThreadState *); +PyAPI_FUNC(void) _PyThreadState_DeleteExcept(PyThreadState *tstate); #ifdef WITH_THREAD PyAPI_FUNC(void) PyThreadState_DeleteCurrent(void); PyAPI_FUNC(void) _PyGILState_Reinit(void); diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 9ed6ca7a168..ecf22fc7494 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -728,6 +728,31 @@ class ThreadJoinOnShutdown(BaseTestCase): for t in threads: t.join() + @unittest.skipUnless(hasattr(os, 'fork'), "needs os.fork()") + def test_clear_threads_states_after_fork(self): + # Issue #17094: check that threads states are cleared after fork() + + # start a bunch of threads + threads = [] + for i in range(16): + t = threading.Thread(target=lambda : time.sleep(0.3)) + threads.append(t) + t.start() + + pid = os.fork() + if pid == 0: + # check that threads states have been cleared + if len(sys._current_frames()) == 1: + os._exit(0) + else: + os._exit(1) + else: + _, status = os.waitpid(pid, 0) + self.assertEqual(0, status) + + for t in threads: + t.join() + class ThreadingExceptionTests(BaseTestCase): # A RuntimeError should be raised if Thread.start() is called diff --git a/Misc/NEWS b/Misc/NEWS index 70e8f2130a4..0ff2e17e1c4 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,11 @@ What's New in Python 3.4.0 Alpha 1? Core and Builtins ----------------- +- Issue #17094: Clear stale thread states after fork(). Note that this + is a potentially disruptive change since it may release some system + resources which would otherwise remain perpetually alive (e.g. database + connections kept in thread-local storage). + - Issue #17408: Avoid using an obsolete instance of the copyreg module when the interpreter is shutdown and then started again. diff --git a/Python/ceval.c b/Python/ceval.c index cbc0fabd032..d32b6fbf584 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -362,29 +362,28 @@ PyEval_ReleaseThread(PyThreadState *tstate) drop_gil(tstate); } -/* This function is called from PyOS_AfterFork to ensure that newly - created child processes don't hold locks referring to threads which - are not running in the child process. (This could also be done using - pthread_atfork mechanism, at least for the pthreads implementation.) */ +/* This function is called from PyOS_AfterFork to destroy all threads which are + * not running in the child process, and clear internal locks which might be + * held by those threads. (This could also be done using pthread_atfork + * mechanism, at least for the pthreads implementation.) */ void PyEval_ReInitThreads(void) { _Py_IDENTIFIER(_after_fork); PyObject *threading, *result; - PyThreadState *tstate = PyThreadState_GET(); + PyThreadState *current_tstate = PyThreadState_GET(); if (!gil_created()) return; recreate_gil(); pending_lock = PyThread_allocate_lock(); - take_gil(tstate); + take_gil(current_tstate); main_thread = PyThread_get_thread_ident(); /* Update the threading module with the new state. */ - tstate = PyThreadState_GET(); - threading = PyMapping_GetItemString(tstate->interp->modules, + threading = PyMapping_GetItemString(current_tstate->interp->modules, "threading"); if (threading == NULL) { /* threading not imported */ @@ -397,6 +396,9 @@ PyEval_ReInitThreads(void) else Py_DECREF(result); Py_DECREF(threading); + + /* Destroy all threads except the current one */ + _PyThreadState_DeleteExcept(current_tstate); } #else diff --git a/Python/pystate.c b/Python/pystate.c index 70038936d67..2a6f16c87f1 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -414,6 +414,53 @@ PyThreadState_DeleteCurrent() #endif /* WITH_THREAD */ +/* + * Delete all thread states except the one passed as argument. + * Note that, if there is a current thread state, it *must* be the one + * passed as argument. Also, this won't touch any other interpreters + * than the current one, since we don't know which thread state should + * be kept in those other interpreteres. + */ +void +_PyThreadState_DeleteExcept(PyThreadState *tstate) +{ + PyInterpreterState *interp = tstate->interp; + PyThreadState *p, *next, *garbage; + HEAD_LOCK(); + /* Remove all thread states, except tstate, from the linked list of + thread states. This will allow calling PyThreadState_Clear() + without holding the lock. + XXX This would be simpler with a doubly-linked list. */ + garbage = interp->tstate_head; + interp->tstate_head = tstate; + if (garbage == tstate) { + garbage = garbage->next; + tstate->next = NULL; + } + else { + for (p = garbage; p; p = p->next) { + if (p->next == tstate) { + p->next = tstate->next; + tstate->next = NULL; + break; + } + } + } + if (tstate->next != NULL) + Py_FatalError("_PyThreadState_DeleteExcept: tstate not found " + "in interpreter thread states"); + HEAD_UNLOCK(); + /* Clear and deallocate all stale thread states. Even if this + executes Python code, we should be safe since it executes + in the current thread, not one of the stale threads. */ + for (p = garbage; p; p = next) { + next = p->next; + PyThreadState_Clear(p); + free(p); + } +} + + PyThreadState * PyThreadState_Get(void) {