Re: [Lldb-commits] [PATCH] D24850: add hook for calling platform-dependent pre-kill action on a timed out test
tfiala closed this revision. tfiala added a comment. Closed by r282258 https://reviews.llvm.org/D24850 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24850: add hook for calling platform-dependent pre-kill action on a timed out test
tfiala updated this revision to Diff 72294.
tfiala added a comment.
I'm about to check this in. I just wanted to put up my final change, which
includes the following:
- README.md - documents the new pre_kill_hook package in lldbsuite.
- added a runner_context/context_dict argument to the pre-kill-hook logic.
This dictionary will contain:
- entry 'args': array containing configuration.args
- entry 'platform_name': contains configuration.lldb_platform_name
- entry 'platform_url': contains configuration.lldb_platform_url
- entry 'platform_working_dir': contains
configuration.lldb_platform_working_dir
I pass the dictionary in to decouple the pre_kill_hook package from the test
runner configuration module. (Also, the configuration module logic is not
guaranteed to be run on any of those test queue workers, which may be in
separate processes when using the multiprocessing* test runner strategies).
https://reviews.llvm.org/D24850
Files:
packages/Python/lldbsuite/pre_kill_hook/README.md
packages/Python/lldbsuite/pre_kill_hook/__init__.py
packages/Python/lldbsuite/pre_kill_hook/darwin.py
packages/Python/lldbsuite/pre_kill_hook/tests/__init__.py
packages/Python/lldbsuite/pre_kill_hook/tests/test_darwin.py
packages/Python/lldbsuite/test/dosep.py
packages/Python/lldbsuite/test/test_runner/process_control.py
Index: packages/Python/lldbsuite/test/test_runner/process_control.py
===
--- packages/Python/lldbsuite/test/test_runner/process_control.py
+++ packages/Python/lldbsuite/test/test_runner/process_control.py
@@ -483,6 +483,19 @@
def on_process_exited(self, command, output, was_timeout, exit_status):
pass
+def on_timeout_pre_kill(self):
+"""Called after the timeout interval elapses but before killing it.
+
+This method is added to enable derived classes the ability to do
+something to the process prior to it being killed. For example,
+this would be a good spot to run a program that samples the process
+to see what it was doing (or not doing).
+
+Do not attempt to reap the process (i.e. use wait()) in this method.
+That will interfere with the kill mechanism and return code processing.
+"""
+pass
+
def write(self, content):
# pylint: disable=no-self-use
# Intended - we want derived classes to be able to override
@@ -640,6 +653,11 @@
# Reap the child process here.
self.returncode = self.process.wait()
else:
+
+# Allow derived classes to do some work after we detected
+# a timeout but before we touch the timed-out process.
+self.on_timeout_pre_kill()
+
# Prepare to stop the process
process_terminated = completed_normally
terminate_attempt_count = 0
Index: packages/Python/lldbsuite/test/dosep.py
===
--- packages/Python/lldbsuite/test/dosep.py
+++ packages/Python/lldbsuite/test/dosep.py
@@ -46,6 +46,7 @@
import sys
import threading
+from six import StringIO
from six.moves import queue
# Our packages and modules
@@ -64,6 +65,8 @@
# Status codes for running command with timeout.
eTimedOut, ePassed, eFailed = 124, 0, 1
+g_session_dir = None
+g_runner_context = None
output_lock = None
test_counter = None
total_tests = None
@@ -227,6 +230,39 @@
failures,
unexpected_successes)
+def on_timeout_pre_kill(self):
+# We're just about to have a timeout take effect. Here's our chance
+# to do a pre-kill action.
+
+# For now, we look to see if the lldbsuite.pre_kill module has a
+# runner for our platform.
+module_name = "lldbsuite.pre_kill_hook." + platform.system().lower()
+import importlib
+try:
+module = importlib.import_module(module_name)
+except ImportError:
+# We don't have one for this platform. Skip.
+sys.stderr.write("\nwarning: no timeout handler module: " +
+ module_name)
+return
+
+# Try to run the pre-kill-hook method.
+try:
+# Run the pre-kill command.
+output_io = StringIO()
+module.do_pre_kill(self.pid, g_runner_context, output_io)
+
+# Write the output to a filename associated with the test file and
+# pid.
+basename = "{}-{}.sample".format(self.file_name, self.pid)
+sample_path = os.path.join(g_session_dir, basename)
+with open(sample_path, "w") as output_file:
+output_file.write(output_io.getvalue())
+except Exception as e:
+sys.stderr.write("caught exception while running "
+ "pre-kill action: {}".format(e))
+return
+
def is_exceptional_exit(self):
Re: [Lldb-commits] [PATCH] D24850: add hook for calling platform-dependent pre-kill action on a timed out test
tfiala added inline comments. Comment at: packages/Python/lldbsuite/test/test_runner/process_control.py:514 @@ +513,3 @@ +# to it later. +self.command = command + labath wrote: > Is this actually used anywhere? No - I originally was parsing some options out of it in the handler, but I no longer am doing that. I will take this out of the final. It's easy enough to add in later if we ever need it. https://reviews.llvm.org/D24850 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24850: add hook for calling platform-dependent pre-kill action on a timed out test
labath accepted this revision. labath added a comment. Sounds like a useful thing to have. I've found turning on logging very helpful when looking for these issues, as it can tell you what was happening in the past, in addition to the current state (also it allows you to compare the logs from a successful and unsuccessful run). Comment at: packages/Python/lldbsuite/test/test_runner/process_control.py:514 @@ +513,3 @@ +# to it later. +self.command = command + Is this actually used anywhere? https://reviews.llvm.org/D24850 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24850: add hook for calling platform-dependent pre-kill action on a timed out test
tfiala added a comment. Greg also had the idea of having a fallback mechanism that uses a newly-spun-up lldb to attach to the to-be-killed process, and retrieves the threads and backdtraces, to dump out a compact description. That's nice in that it should work on any host that has a working lldb with python support. https://reviews.llvm.org/D24850 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24850: add hook for calling platform-dependent pre-kill action on a timed out test
tfiala added inline comments.
Comment at: packages/Python/lldbsuite/test/dosep.py:238-245
@@ +237,10 @@
+# runner for our platform.
+module_name = "lldbsuite.pre_kill_hook." + platform.system().lower()
+try:
+import importlib
+module = importlib.import_module(module_name)
+except ImportError:
+# We don't have one for this platform. Skip.
+sys.stderr.write("\nwarning: no timeout handler module: " +
module_name)
+return
+
I suspect we will need to tweak this a bit. We need to be able to dispatch on
more than just the host platform.system(). It may be sufficient to pass along
the test platform info as an argument.
https://reviews.llvm.org/D24850
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D24850: add hook for calling platform-dependent pre-kill action on a timed out test
tfiala created this revision.
tfiala added reviewers: clayborg, labath.
tfiala added a subscriber: lldb-commits.
This change introduces the concept of a platform-specific, pre-kill-hook
mechanism. If a platform defines the hook, then the hook gets called right
after a timeout is detected in a test run, but before the process is killed.
The pre-kill-hook mechanism works as follows:
* When a timeout is detected in the process_control.ProcessDriver class that
runs the per-test lldb process, a new overridable on_timeout_pre_kill() method
is called on the ProcessDriver instance.
* The concurrent test driver's derived ProcessDriver overrides this method. It
looks to see if a module called
"lldbsuite.pre_kill_hook/{platform-system-name}" module exists, where
platform-system-name is replaced with platform.system().lower():
* If that module doesn't exist, no further process of the pre-killed process
occurs.
* If that module does exist, it is loaded, and the method
"do_pre_kill(process_id, output_stream)" is called. If that method throws an
exception, we log it and we ignore further processing of the pre-killed process.
* The process_id arg of the do_pre_kill function is the process id as
returned by the ProcessDriver.pid property.
* The output_stream arg of the do_pre_kill function takes a file-like object.
Output to be collected from doing an processing on the process-to-be-killed
should be written into the object. The current impl uses a six.StringIO and
then writes this output to {TestFilename}-{pid}.sample in the session directory.
Platforms where platform.system() is "Darwin" will get a pre-kill action that
runs the 'sample' program on the lldb that has timed out. That data will be
collected on CI and analyzed to determine what is happening during timeouts.
(This has an advantage over a core in that it is much smaller and that it
clearly demonstrates any liveness of the process, if there is any).
I will also hunt around on Linux to see if there might be something akin to
'sample' that might be available. If so, it would be nice to hook something up
for that.
https://reviews.llvm.org/D24850
Files:
packages/Python/lldbsuite/pre_kill_hook/__init__.py
packages/Python/lldbsuite/pre_kill_hook/darwin.py
packages/Python/lldbsuite/pre_kill_hook/tests/__init__.py
packages/Python/lldbsuite/pre_kill_hook/tests/test_darwin.py
packages/Python/lldbsuite/test/dosep.py
packages/Python/lldbsuite/test/test_runner/process_control.py
Index: packages/Python/lldbsuite/test/test_runner/process_control.py
===
--- packages/Python/lldbsuite/test/test_runner/process_control.py
+++ packages/Python/lldbsuite/test/test_runner/process_control.py
@@ -472,6 +472,7 @@
# be fast.
self.hard_terminate_timeout = 5.0
self.returncode = None
+self.command = None
# =
# Methods for subclasses to override if desired.
@@ -483,6 +484,19 @@
def on_process_exited(self, command, output, was_timeout, exit_status):
pass
+def on_timeout_pre_kill(self):
+"""Called after the timeout interval elapses but before killing it.
+
+This method is added to enable derived classes the ability to do
+something to the process prior to it being killed. For example,
+this would be a good spot to run a program that samples the process
+to see what it was doing (or not doing).
+
+Do not attempt to reap the process (i.e. use wait()) in this method.
+That will interfere with the kill mechanism and return code processing.
+"""
+pass
+
def write(self, content):
# pylint: disable=no-self-use
# Intended - we want derived classes to be able to override
@@ -495,6 +509,10 @@
# ==
def run_command(self, command):
+# Remember the command we were running so we can refer back
+# to it later.
+self.command = command
+
# Start up the child process and the thread that does the
# communication pump.
self._start_process_and_io_thread(command)
@@ -640,6 +658,11 @@
# Reap the child process here.
self.returncode = self.process.wait()
else:
+
+# Allow derived classes to do some work after we detected
+# a timeout but before we touch the timed-out process.
+self.on_timeout_pre_kill()
+
# Prepare to stop the process
process_terminated = completed_normally
terminate_attempt_count = 0
Index: packages/Python/lldbsuite/test/dosep.py
===
--- packages/Python/lldbsuite/test/dosep.py
+++ packages/Python/lldbsuite/test/dosep.py
@@ -46,6 +46,7 @@
import sys
import threading
+from six import
