From 1d250001f5d298ec2e295aa9dadbad008eac3939 Mon Sep 17 00:00:00 2001 From: Tim Peters Date: Tue, 28 Mar 2006 07:09:33 +0000 Subject: Merge rev 43181 from the trunk. Try to repair at least one segfault on the Mac buildbot, as diagnosed by Nick Coghlan. test_capi.py: A test module should never spawn a thread as a side effect of being imported. Because this one did, the segfault one of its thread tests caused didn't occur until a few tests after test_regrtest.py thought test_capi was finished. Repair that. Also join() the thread spawned at the end, so that test_capi is truly finished when regrtest reports that it's done. _testcapimodule.c test_thread_state(): this spawns a couple of non-threading.py threads, passing them a PyObject* argument, but did nothing to ensure that those threads finished before returning. As a result, the PyObject* _could_ (although this was unlikely) get decref'ed out of existence before the threads got around to using it. Added explicit synchronization (via a Python mutex) so that test_thread_state can reliably wait for its spawned threads to finish. --- Lib/test/test_capi.py | 87 +++++++++++++++++++++++++---------------------- Modules/_testcapimodule.c | 49 +++++++++++++++++++++----- 2 files changed, 87 insertions(+), 49 deletions(-) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 1dd2461..cdd84bb 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -5,44 +5,51 @@ import sys from test import test_support import _testcapi -for name in dir(_testcapi): - if name.startswith('test_'): - test = getattr(_testcapi, name) +def test_main(): + + for name in dir(_testcapi): + if name.startswith('test_'): + test = getattr(_testcapi, name) + if test_support.verbose: + print "internal", name + try: + test() + except _testcapi.error: + raise test_support.TestFailed, sys.exc_info()[1] + + # some extra thread-state tests driven via _testcapi + def TestThreadState(): + import thread + import time + if test_support.verbose: - print "internal", name - try: - test() - except _testcapi.error: - raise test_support.TestFailed, sys.exc_info()[1] - -# some extra thread-state tests driven via _testcapi -def TestThreadState(): - import thread - import time - - if test_support.verbose: - print "auto-thread-state" - - idents = [] - - def callback(): - idents.append(thread.get_ident()) - - _testcapi._test_thread_state(callback) - time.sleep(1) - # Check our main thread is in the list exactly 3 times. - if idents.count(thread.get_ident()) != 3: - raise test_support.TestFailed, \ - "Couldn't find main thread correctly in the list" - -try: - _testcapi._test_thread_state - have_thread_state = True -except AttributeError: - have_thread_state = False - -if have_thread_state: - TestThreadState() - import threading - t=threading.Thread(target=TestThreadState) - t.start() + print "auto-thread-state" + + idents = [] + + def callback(): + idents.append(thread.get_ident()) + + _testcapi._test_thread_state(callback) + a = b = callback + time.sleep(1) + # Check our main thread is in the list exactly 3 times. + if idents.count(thread.get_ident()) != 3: + raise test_support.TestFailed, \ + "Couldn't find main thread correctly in the list" + + try: + _testcapi._test_thread_state + have_thread_state = True + except AttributeError: + have_thread_state = False + + if have_thread_state: + TestThreadState() + import threading + t=threading.Thread(target=TestThreadState) + t.start() + t.join() + +if __name__ == "__main__": + test_main() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index d842b23..6ac2572 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -562,7 +562,17 @@ raise_exception(PyObject *self, PyObject *args) #ifdef WITH_THREAD -void _make_call(void *callable) +/* test_thread_state spawns a thread of its own, and that thread releases + * `thread_done` when it's finished. The driver code has to know when the + * thread finishes, because the thread uses a PyObject (the callable) that + * may go away when the driver finishes. The former lack of this explicit + * synchronization caused rare segfaults, so rare that they were seen only + * on a Mac buildbot (although they were possible on any box). + */ +static PyThread_type_lock thread_done = NULL; + +static void +_make_call(void *callable) { PyObject *rc; PyGILState_STATE s = PyGILState_Ensure(); @@ -571,32 +581,53 @@ void _make_call(void *callable) PyGILState_Release(s); } +/* Same thing, but releases `thread_done` when it returns. This variant + * should be called only from threads spawned by test_thread_state(). + */ +static void +_make_call_from_thread(void *callable) +{ + _make_call(callable); + PyThread_release_lock(thread_done); +} + static PyObject * test_thread_state(PyObject *self, PyObject *args) { PyObject *fn; + if (!PyArg_ParseTuple(args, "O:test_thread_state", &fn)) return NULL; - /* Ensure Python is setup for threading */ + + /* Ensure Python is set up for threading */ PyEval_InitThreads(); - /* Start a new thread for our callback. */ - PyThread_start_new_thread( _make_call, fn); + thread_done = PyThread_allocate_lock(); + if (thread_done == NULL) + return PyErr_NoMemory(); + PyThread_acquire_lock(thread_done, 1); + + /* Start a new thread with our callback. */ + PyThread_start_new_thread(_make_call_from_thread, fn); /* Make the callback with the thread lock held by this thread */ _make_call(fn); /* Do it all again, but this time with the thread-lock released */ Py_BEGIN_ALLOW_THREADS _make_call(fn); + PyThread_acquire_lock(thread_done, 1); /* wait for thread to finish */ Py_END_ALLOW_THREADS + /* And once more with and without a thread - XXX - should use a lock and work out exactly what we are trying - to test + XXX - should use a lock and work out exactly what we are trying + to test */ Py_BEGIN_ALLOW_THREADS - PyThread_start_new_thread( _make_call, fn); + PyThread_start_new_thread(_make_call_from_thread, fn); _make_call(fn); + PyThread_acquire_lock(thread_done, 1); /* wait for thread to finish */ Py_END_ALLOW_THREADS - Py_INCREF(Py_None); - return Py_None; + + PyThread_free_lock(thread_done); + Py_RETURN_NONE; } #endif -- cgit v0.12