[Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism

[Todd Fiala via lldb-commits]

tfiala added a comment.

In http://reviews.llvm.org/D13124#255034, @zturner wrote:

> As far as I can tell, the value of the return code is undocumented when you
>  use Popen.terminate() on Windows.


Okay, interesting.  I found what looked like a race on Linux and OS X where calling the Popen-object returncode member (the normal way to get the returncode) would flip from the actual result (received after a wait() call, i.e.

popen_object.wait()
self.returncode = popen_object.returncode

...
returncode2 = popen_object.returncode

and find
returncode2 != self.returncode

I switched to using the result of wait() as the official return code.

I don't know what that means for the

> patch.  It's quite a bit more complicated than I anticipated based on the

>  original thread to lldb-dev.  I thought it was just going to call

>  Popen.terminate and assume that means it was a hard terminate.  I still

>  don't grasp the need for all the complexity.


It's the need for the core file generation, which requires a soft (i.e. deflectable by the target process) terminate request.  Which requires a 2-level kill mechanism, as we need to make sure the process really dies for a real test runner environment that needs to be long-lived.

I can do one more adjustment.  I'll make the soft terminate optional per platform.  We'll disable it on Windows and only enable it on Posixy stuff.  Then on Windows there will only be the hard terminate.

> A few experiments on a throwaway program suggest that using

>  Popen.terminate() sets returncode to 1, but there doesn't seem to be any

>  guarantee that this is always the case.


What I was doing was writing a value into the subprocess.Popen-like object on the Posix side, to store extra state needed.  (e.g. whether we're killing by process group --- if we created a process group --- or if we're just killing a process directly.  Those are different operations on the Posix side).  For this, you could just write into the popen object that you killed it with a hard terminate.  And then read back that value on the verification side to say "oh yes, that was really the mechanism by which it  was killed."

That point is moot, though, if there is only one way to kill on Windows.  If there is only one way to kill, then one of these should happen:

1. If we don't care about crashdumps on Windows, which I think I heard you say is true, then we make the soft terminate level optional by platform, and skip it (i.e. indicate it is not available) on Windows.

2. If we do care about crashdumps on Windows, then we probably need to untangle the conflation of the soft terminate level (where a target process can ignore) from crash dump generation.  On Posix, crash dump request is tied to the soft termination signal used.  But on Windows, we could allow that to happen on the (one and only) hard terminate level.

I can get that together (assuming option #1) relatively quickly.

I don't think we can drop the complexity of the 2-tier kill level in the driver, though, given the desire to support core dump generation.


http://reviews.llvm.org/D13124