Community
Participate
Working Groups
java.lang.Process.destroy() is meant to forcibly terminate a process. HostShellProcessAdapter.destroy() does not forcibly terminate the process however. Instead, it calls IHostShell.exit(), which sends the text "exit" to the shell to cause it to, well, exit. The upshot of this is that it actually waits for any running processes to finish before the exit command is sent (or more correctly, the shell doesn't get the exit command until its foreground process finishes). This makes HostShellProcessAdapter.destroy() pretty useless. We are using this currently to manage remote processes in PTP, but this means we cannot kill them, so we cannot do things like cancel a remote build that is in progress. If there is a better way to manage remote processes that avoids this issue we would be open to switching to it.
I changed the Summary, previous value was: HostShellProcessAdapter.destroy() does not follow semantics of Process.destroy() because the issue is dstore specific: looking at SshHostShell, for instance, you'll see that calling its #exit() method does forcefully close the Streams and thus terminate the shell synchronously as you expect. Arguably, exact API specification of IHostShell#exit() behavior is not yet documented, and I just filed bug 244130 to get this done. Looking at DStoreShellThread and CommandMinerThread, I think it should work for now if you send the command "#break" in order to send a break signal that should kill your child process. Let me know if this works for you, then we can rev down severity and think about sending a #break in DStoreHostShell's exit() method automatically in order to make behavior of the various IHostShell implementations work the same. The HostShellProcessAdapter does work as expected IMHO, assuming that IHostShell#exit() must terminate child processes. Yet another aspect of this bug is that when we launch something from a Shell, we currently have no chance getting the PID of the launched process in a Platform independent manner, such that we could use the IProcessService to actually kill the process. Bug 161774 has been open for a long time to investigate this. Your input and, in the ideal case, a patch would be highly appreciated for fixing this since you obviously have an environment to work on this.
Yet another aspect of this is the need for public API to send signals to IHostShell instances. I filed bug 244132 to track this.
I'm inclined to rev down severity since my questions from comment 1 were not answered, specifically whether sending the command "#break" helps. I'm not sure who can work on this without your help. The hints should be sufficient to come up with a patch.
(In reply to comment #3) > I'm inclined to rev down severity since my questions from comment 1 were not > answered, specifically whether sending the command "#break" helps. I'm not sure > who can work on this without your help. The hints should be sufficient to come > up with a patch. Well, we've been busy getting RDT out the door, and now am on a long overdue vacation, so I haven't had time to try this yet. I will take a look at some point after I'm back.
(In reply to comment #3) > I'm inclined to rev down severity since my questions from comment 1 were not > answered, specifically whether sending the command "#break" helps. I'm not sure > who can work on this without your help. The hints should be sufficient to come > up with a patch. I tried sending the "#break" in DStoreHostShell.exit(), but it had no effect. I assume you meant to literally send those characters and that you did not mean some shorthand for CTRL + BREAK.
Chris - as per bug 153275 comment 1 it looks like you might need a PTY on the remote to be able and respond to the #break command. The PTY is only IBM closed source for now. Bug 196337 has some PTY code attached under EPL, but I'm not sure if that will work here. Dave M, do you have an idea how dstore could forcefully terminate child processes on IHostShell#exit() ? -- Since the dstore miner is getting a Process object back from the Runtime.exec() command it should work to just Process.destroy() when an exit command is received?
Created attachment 124595 [details] patch to issue cancel command on DStoreHostShell.exist()
(In reply to comment #6) > Chris - as per bug 153275 comment 1 it looks like you might need a PTY on the > remote to be able and respond to the #break command. The PTY is only IBM closed > source for now. Bug 196337 has some PTY code attached under EPL, but I'm not > sure if that will work here. > > Dave M, do you have an idea how dstore could forcefully terminate child > processes on IHostShell#exit() ? -- Since the dstore miner is getting a Process > object back from the Runtime.exec() command it should work to just > Process.destroy() when an exit command is received? > With the patch I just attached, we issue a cancel command instead of writing "exit". On the server side this first issues an "exit" but if that fails, it falls back to a Process.destroy(). Does this help?
(In reply to comment #8) > With the patch I just attached, we issue a cancel command instead of writing > "exit". On the server side this first issues an "exit" but if that fails, it > falls back to a Process.destroy(). Does this help? Works :-)
I've committed the change to cvs.
Is there any chance of this being backported to 3.0.3? Although 3.1 seems so far compatible with Eclipse 3.4, it is a long time until June and I would really like it if our users could cancel builds before then (see bug 250319).
(In reply to comment #11) > Is there any chance of this being backported to 3.0.3? Although 3.1 seems so > far compatible with Eclipse 3.4, it is a long time until June and I would > really like it if our users could cancel builds before then (see bug 250319). > I've opened bug 250319 for the backport to 3.0.3.
(In reply to comment #12) > I've opened bug 250319 for the backport to 3.0.3. I think you mean bug 263669
(In reply to comment #13) > (In reply to comment #12) > > I've opened bug 250319 for the backport to 3.0.3. > > I think you mean bug 263669 > Oh right, that's what I meant.
