[WIP][2.7] bpo-30351: regrtest: add --timeout#2317
[WIP][2.7] bpo-30351: regrtest: add --timeout#2317vstinner wants to merge 1 commit intopython:2.7python/cpython:2.7from vstinner:watchdogCopy head branch name to clipboard
Conversation
|
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 :-/ |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
Why twice? Maybe sleep between them?
| try: | ||
| self.dump_threads() | ||
| except: | ||
| self.write("FAILED TO DUMP THREADS") |
| try: | ||
| self.dump_thread(stack) | ||
| except: | ||
| self.write("FAILED TO DUMP THREAD STACK") |
| if not self.is_alive(): | ||
| return | ||
|
|
||
| print("Stop watchdog") |
| def __init__(self, timeout): | ||
| threading.Thread.__init__(self) | ||
| self.timeout = timeout | ||
| self.stream = sys.__stdout__ |
There was a problem hiding this comment.
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 ***") |
There was a problem hiding this comment.
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") |
There was a problem hiding this comment.
Maybe print >>self.stream, line? This works even if self.stream is None.
|
|
||
| print("Stop watchdog") | ||
| self.quit = True | ||
| self.join() |
There was a problem hiding this comment.
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. |
|
I rebased my PR: 3019. |
|
PR #3019. |
Add an optional watchdog thread which dumps the Python traceback
regrtest takes longer than timeout seconds.