Summary: | [webkitpy] Use existing signal handler when printing stacktrace | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jonathan Bedard <jbedard> | ||||||||
Component: | Tools / Tests | Assignee: | Jonathan Bedard <jbedard> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aakash_jain, dewei_zhu, ews-watchlist, glenn, slewis, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=226234 | ||||||||||
Attachments: |
|
Description
Jonathan Bedard
2021-05-25 13:00:03 PDT
Created attachment 429686 [details]
Patch
Comment on attachment 429686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429686&action=review > Tools/Scripts/webkitpy/common/interrupt_debugging.py:95 > def log_stack_trace_on_term(output_file=None): > + if not getattr(signal, 'SIGTERM'): > + return > + > + previous = signal.getsignal(signal.SIGTERM) > + > def handler(signum, frame): > with StackTraceFileContext(output_file=output_file) as file: > file.write('SIGTERM signal received') > log_stack_trace(frame, file) > > - exit(-1) > + return previous(signum, frame) > > - signal.signal(signal.SIGTERM, handler) > + return signal.signal(signal.SIGTERM, handler) > > > def log_stack_trace_on_ctrl_c(output_file=None): > + if not getattr(signal, 'SIGINT'): > + return > + > + previous = signal.getsignal(signal.SIGINT) > + > def handler(signum, frame): > with StackTraceFileContext(output_file=output_file) as file: > file.write('CTRL+C received\n') > log_stack_trace(frame, file) > > - raise KeyboardInterrupt > + return previous(signum, frame) > > - signal.signal(signal.SIGINT, handler) > + return signal.signal(signal.SIGINT, handler) Those two are almost identical, maybe we can do some refactor here? Comment on attachment 429686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429686&action=review >> Tools/Scripts/webkitpy/common/interrupt_debugging.py:95 >> + return signal.signal(signal.SIGINT, handler) > > Those two are almost identical, maybe we can do some refactor here? We could convert them both to "log_stack_trace_on_signal" Created attachment 429950 [details]
Patch
Comment on attachment 429950 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429950&action=review r=me > Tools/Scripts/webkitpy/common/interrupt_debugging.py:66 > +def log_stack_trace_on_signal(signum, output_file=None): Would it a better logging info if we pass sig_name instead of signum? log_stack_trace_on_signal('SIGTERM', output_file=stack_trace_path) log_stack_trace_on_signal('SIGINT', output_file=stack_trace_path) Comment on attachment 429950 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429950&action=review > Tools/Scripts/webkitpy/common/interrupt_debugging.py:83 > + if previous == getattr(signal, 'SIG_DFL'): > + if signum_ == getattr(signal, 'SIGTERM'): > + sys.exit(-1) > + if signum_ == getattr(signal, 'SIGINT'): > + raise KeyboardInterrupt In previous version patch, we don't do those? (In reply to dewei_zhu from comment #7) > Comment on attachment 429950 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=429950&action=review > > > Tools/Scripts/webkitpy/common/interrupt_debugging.py:83 > > + if previous == getattr(signal, 'SIG_DFL'): > > + if signum_ == getattr(signal, 'SIGTERM'): > > + sys.exit(-1) > > + if signum_ == getattr(signal, 'SIGINT'): > > + raise KeyboardInterrupt > > In previous version patch, we don't do those? No, I realized (with the keyboard interrupt, specifically), the only reason it was "working" is that attempting to invoke an integer resulted in an exception. (In reply to dewei_zhu from comment #6) > Comment on attachment 429950 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=429950&action=review > > r=me > > > Tools/Scripts/webkitpy/common/interrupt_debugging.py:66 > > +def log_stack_trace_on_signal(signum, output_file=None): > > Would it a better logging info if we pass sig_name instead of signum? > > log_stack_trace_on_signal('SIGTERM', output_file=stack_trace_path) > log_stack_trace_on_signal('SIGINT', output_file=stack_trace_path) Good idea! Let me update for this! Created attachment 429964 [details]
Patch for landing
Committed r278184 (238227@main): <https://commits.webkit.org/238227@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 429964 [details]. |