[WIP][2.7] bpo-30351: regrtest: add --timeout by vstinner · Pull Request #2317 · python/cpython
Add an optional watchdog thread which dumps the Python traceback
regrtest takes longer than timeout seconds.
This is an experimental debug tool for regrtest to try to debug http://bugs.python.org/issue30351 since faulthandler is not in the Python 2.7 stdlib (it was added to Python 3.3), and it's not that easy to install a C extension from PyPI on buildbots.
Add an optional watchdog thread which dumps the Python traceback regrtest takes longer than timeout seconds.
Using a thread is portable, but Python 2.7 doesn't have signal.pthread_sigmask(), so the thread may receive signals which is likely to change the behaviour of some tests relying on signals :-/
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not very experienced with all this, but LGTM. I have added a couple of nitpicks, questions and suggestions, but in any case approve the PR.
|
|
||
| if watchdog is not None: | ||
| watchdog.stop() | ||
| watchdog.stop() |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why twice? Maybe sleep between them?
| try: | ||
| self.dump_threads() | ||
| except: | ||
| self.write("FAILED TO DUMP THREADS") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Print a traceback?
| try: | ||
| self.dump_thread(stack) | ||
| except: | ||
| self.write("FAILED TO DUMP THREAD STACK") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Print a traceback?
| if not self.is_alive(): | ||
| return | ||
|
|
||
| print("Stop watchdog") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.write()?
| def __init__(self, timeout): | ||
| threading.Thread.__init__(self) | ||
| self.timeout = timeout | ||
| self.stream = sys.__stdout__ |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't __stderr__ be more appropriate for debug and error messages?
Can't standard streams be None on Windows?
| self.write(" %s" % line) | ||
|
|
||
| def dump_threads(self): | ||
| self.write("*** STACKTRACE - START ***") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the case if the previous outputted line is not ended with a newline and for attracting an attention add a \n or two before the message.
| self.deadline = time.time() + self.timeout | ||
|
|
||
| def write(self, line): | ||
| self.stream.write(line + "\n") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe print >>self.stream, line? This works even if self.stream is None.
|
|
||
| print("Stop watchdog") | ||
| self.quit = True | ||
| self.join() |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
join() takes the timeout argument. Is it worth to use it?
The design of this debug tool is very different from faulthandler and is likely to cause bugs when it's enabled. I'm not sure that I would like to get this change merged :-(
I wrote it to try to debug regrtest hangs on Python 2.7 on platforms without gdb with python-gdb.py plugin.
You better know what your code does. 😉
I don't think that the watchdog is reliable, it can "catch" signals (we miss signal.pthread_sigmask() on Python 2.7), and 2.7 buildbots became very stable so I don't need this tool anymore. I just abandon my PR.