[Python-checkins] cpython: faulthandler: one more time, fix usage of locks in the watchdog thread

Fri Apr 8 12:59:15 CEST 2011

http://hg.python.org/cpython/rev/e409d9beaf6b
changeset: 69206:e409d9beaf6b
user: Victor Stinner <victor.stinner at haypocalc.com>
date: Fri Apr 08 12:57:06 2011 +0200
summary:
 faulthandler: one more time, fix usage of locks in the watchdog thread
 * Write a new test to ensure that dump_tracebacks_later() still works if
 it was already called and then cancelled before
 * Don't use a variable to check the status of the thread, only rely on locks
 * The thread only releases cancel_event if it was able to acquire it (if
 the timer was interrupted)
 * The main thread always hold this lock. It is only released when
 faulthandler_thread() is interrupted until this thread exits, or at Python
 exit.
files:
 Lib/test/test_faulthandler.py | 45 ++++++++++-------
 Modules/faulthandler.c | 57 +++++++++++-----------
 2 files changed, 55 insertions(+), 47 deletions(-)

diff --git a/Lib/test/test_faulthandler.py b/Lib/test/test_faulthandler.py
--- a/Lib/test/test_faulthandler.py
+++ b/Lib/test/test_faulthandler.py
@@ -352,7 +352,7 @@
 with temporary_filename() as filename:
 self.check_dump_traceback_threads(filename)
 
- def _check_dump_tracebacks_later(self, repeat, cancel, filename):
+ def _check_dump_tracebacks_later(self, repeat, cancel, filename, loops):
 """
 Check how many times the traceback is written in timeout x 2.5 seconds,
 or timeout x 3.5 seconds if cancel is True: 1, 2 or 3 times depending
@@ -364,42 +364,43 @@
 import faulthandler
 import time
 
-def func(repeat, cancel, timeout):
- if cancel:
+def func(timeout, repeat, cancel, file, loops):
+ for loop in range(loops):
+ faulthandler.dump_tracebacks_later(timeout, repeat=repeat, file=file)
+ if cancel:
+ faulthandler.cancel_dump_tracebacks_later()
+ time.sleep(timeout * 2.5)
 faulthandler.cancel_dump_tracebacks_later()
- time.sleep(timeout * 2.5)
- faulthandler.cancel_dump_tracebacks_later()
 
 timeout = {timeout}
 repeat = {repeat}
 cancel = {cancel}
+loops = {loops}
 if {has_filename}:
 file = open({filename}, "wb")
 else:
 file = None
-faulthandler.dump_tracebacks_later(timeout,
- repeat=repeat, file=file)
-func(repeat, cancel, timeout)
+func(timeout, repeat, cancel, file, loops)
 if file is not None:
 file.close()
 """.strip()
 code = code.format(
- filename=repr(filename),
- has_filename=bool(filename),
+ timeout=TIMEOUT,
 repeat=repeat,
 cancel=cancel,
- timeout=TIMEOUT,
+ loops=loops,
+ has_filename=bool(filename),
+ filename=repr(filename),
 )
 trace, exitcode = self.get_output(code, filename)
 trace = '\n'.join(trace)
 
 if not cancel:
+ count = loops
 if repeat:
- count = 2
- else:
- count = 1
+ count *= 2
 header = 'Thread 0x[0-9a-f]+:\n'
- regex = expected_traceback(7, 19, header, count=count)
+ regex = expected_traceback(9, 20, header, count=count)
 self.assertRegex(trace, regex)
 else:
 self.assertEqual(trace, '')
@@ -408,12 +409,17 @@
 @unittest.skipIf(not hasattr(faulthandler, 'dump_tracebacks_later'),
 'need faulthandler.dump_tracebacks_later()')
 def check_dump_tracebacks_later(self, repeat=False, cancel=False,
- file=False):
+ file=False, twice=False):
+ if twice:
+ loops = 2
+ else:
+ loops = 1
 if file:
 with temporary_filename() as filename:
- self._check_dump_tracebacks_later(repeat, cancel, filename)
+ self._check_dump_tracebacks_later(repeat, cancel,
+ filename, loops)
 else:
- self._check_dump_tracebacks_later(repeat, cancel, None)
+ self._check_dump_tracebacks_later(repeat, cancel, None, loops)
 
 def test_dump_tracebacks_later(self):
 self.check_dump_tracebacks_later()
@@ -427,6 +433,9 @@
 def test_dump_tracebacks_later_file(self):
 self.check_dump_tracebacks_later(file=True)
 
+ def test_dump_tracebacks_later_twice(self):
+ self.check_dump_tracebacks_later(twice=True)
+
 @unittest.skipIf(not hasattr(faulthandler, "register"),
 "need faulthandler.register")
 def check_register(self, filename=False, all_threads=False,
diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c
--- a/Modules/faulthandler.c
+++ b/Modules/faulthandler.c
@@ -48,13 +48,14 @@
 int fd;
 PY_TIMEOUT_T timeout_ms; /* timeout in microseconds */
 int repeat;
- int running;
 PyInterpreterState *interp;
 int exit;
- /* released by parent thread when cancel request */
+ /* The main thread always hold this lock. It is only released when
+ faulthandler_thread() is interrupted until this thread exits, or at
+ Python exit. */
 PyThread_type_lock cancel_event;
 /* released by child thread when joined */
- PyThread_type_lock join_event;
+ PyThread_type_lock running;
 } thread;
 #endif
 
@@ -414,7 +415,7 @@
 st = PyThread_acquire_lock_timed(thread.cancel_event,
 thread.timeout_ms, 0);
 if (st == PY_LOCK_ACQUIRED) {
- /* Cancelled by user */
+ PyThread_release_lock(thread.cancel_event);
 break;
 }
 /* Timeout => dump traceback */
@@ -431,21 +432,22 @@
 } while (ok && thread.repeat);
 
 /* The only way out */
- PyThread_release_lock(thread.cancel_event);
- PyThread_release_lock(thread.join_event);
+ PyThread_release_lock(thread.running);
 }
 
 static void
-faulthandler_cancel_dump_tracebacks_later(void)
+cancel_dump_tracebacks_later(void)
 {
- if (thread.running) {
- /* Notify cancellation */
- PyThread_release_lock(thread.cancel_event);
- }
+ /* notify cancellation */
+ PyThread_release_lock(thread.cancel_event);
+
 /* Wait for thread to join */
- PyThread_acquire_lock(thread.join_event, 1);
- PyThread_release_lock(thread.join_event);
- thread.running = 0;
+ PyThread_acquire_lock(thread.running, 1);
+ PyThread_release_lock(thread.running);
+
+ /* The main thread should always hold the cancel_event lock */
+ PyThread_acquire_lock(thread.cancel_event, 1);
+
 Py_CLEAR(thread.file);
 }
 
@@ -489,7 +491,7 @@
 return NULL;
 
 /* Cancel previous thread, if running */
- faulthandler_cancel_dump_tracebacks_later();
+ cancel_dump_tracebacks_later();
 
 Py_XDECREF(thread.file);
 Py_INCREF(file);
@@ -501,14 +503,10 @@
 thread.exit = exit;
 
 /* Arm these locks to serve as events when released */
- PyThread_acquire_lock(thread.join_event, 1);
- PyThread_acquire_lock(thread.cancel_event, 1);
+ PyThread_acquire_lock(thread.running, 1);
 
- thread.running = 1;
 if (PyThread_start_new_thread(faulthandler_thread, NULL) == -1) {
- thread.running = 0;
- PyThread_release_lock(thread.join_event);
- PyThread_release_lock(thread.cancel_event);
+ PyThread_release_lock(thread.running);
 Py_CLEAR(thread.file);
 PyErr_SetString(PyExc_RuntimeError,
 "unable to start watchdog thread");
@@ -521,7 +519,7 @@
 static PyObject*
 faulthandler_cancel_dump_tracebacks_later_py(PyObject *self)
 {
- faulthandler_cancel_dump_tracebacks_later();
+ cancel_dump_tracebacks_later();
 Py_RETURN_NONE;
 }
 #endif /* FAULTHANDLER_LATER */
@@ -1001,15 +999,15 @@
 }
 #endif
 #ifdef FAULTHANDLER_LATER
- thread.running = 0;
 thread.file = NULL;
 thread.cancel_event = PyThread_allocate_lock();
- thread.join_event = PyThread_allocate_lock();
- if (!thread.cancel_event || !thread.join_event) {
+ thread.running = PyThread_allocate_lock();
+ if (!thread.cancel_event || !thread.running) {
 PyErr_SetString(PyExc_RuntimeError,
 "could not allocate locks for faulthandler");
 return -1;
 }
+ PyThread_acquire_lock(thread.cancel_event, 1);
 #endif
 
 return faulthandler_env_options();
@@ -1023,14 +1021,15 @@
 
 #ifdef FAULTHANDLER_LATER
 /* later */
- faulthandler_cancel_dump_tracebacks_later();
+ cancel_dump_tracebacks_later();
 if (thread.cancel_event) {
+ PyThread_release_lock(thread.cancel_event);
 PyThread_free_lock(thread.cancel_event);
 thread.cancel_event = NULL;
 }
- if (thread.join_event) {
- PyThread_free_lock(thread.join_event);
- thread.join_event = NULL;
+ if (thread.running) {
+ PyThread_free_lock(thread.running);
+ thread.running = NULL;
 }
 #endif
 
-- 
Repository URL: http://hg.python.org/cpython
