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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to