[PATCH] D14706: [lit] Implement support of per test timeout in lit.

[Dan Liew via llvm-commits]

delcypher added a comment.

In http://reviews.llvm.org/D14706#303269, @MatzeB wrote:

> This is a good feature to have.


Thanks for taking a look

> I think this implementation will not apply a timeout to the TestRunner.py:executeShCmd codepath (which seems to be a /bin/sh emulation for windows users).


That is incorrect. The timeout is applied on all code paths when the timeout is
enabled. For the `executeShCmd()` (internal shell) case the timeout is
implemented in a different way to `lit.util.executeCommand()` (external shell)
but this definitely works (at least on Linux). I added tests cases for the three
cases: External Shell, Internal Shell and GTest.

> The requirement of an external python package is unfortunate, what about things possible with built-in python libraries such as "os.kill(pid, signal.SIGKILL)" or "resource.setrlimit(resource.RLIMIT_CPU ...)"?


While it is not impossible to remove the dependency on the `pustil` module,
doing so would be difficult due to platform differences. The issue here is
that when killing a process, we must kill the process and its children (and
their children and so on...). If we don't do this we will have processes left
behind that can continue executing past the timeout.

This is even an issue in the most basic case where we use the external shell to
execute a single command because the parent process is the shell and the child
is the actual command we wanted to run.

There are ways to achieve killing of a group of processes on Linux (not sure
about OSX) by having the parent be in its own session (via
`preexe_fn=cos.setsid()`) and then later killing using `os.killpg()` but
that isn't available on Windows and so isn't portable. The aproach I use is
because `pustil` provides a way to easily iterate over a process's children
and kill them. I don't know of any cross platform way of doing this using just
Python's standard library.

I have kept the dependency on `pustil` confined to the
`lit.util.killProcessAndChildren()` function so in the future we could
potentially remove the dependency.  It's not something I can do right now
though as I don't have access to a Windows or OSX machine.


http://reviews.llvm.org/D14706