Looks good! No need for the assignment to p, though.
/Staffan On 25 sep 2014, at 12:39, Jaroslav Bachorik <jaroslav.bacho...@oracle.com> wrote: > On 09/25/2014 12:33 PM, Staffan Larsen wrote: >> >> On 25 sep 2014, at 12:24, Jaroslav Bachorik <jaroslav.bacho...@oracle.com> >> wrote: >> >>> On 09/25/2014 12:13 PM, Staffan Larsen wrote: >>>> I wonder if the p.waitFor() is needed? What if the process launching >>>> expired with a timeout and now we are still waiting for the process to end >>>> - wouldn’t that kind of defeat the timeout? In any case, the >>>> destroyForcibly() should end the process whether we wait for it or not. >>> >>> It would be wonderful but the javadoc states that the result of >>> destroyForcibly() call depends on the implementation and may actually not >>> force close the process and one should use waitFor() to make sure that the >>> process has in fact died. >> >> It also mentions that Processes returned by ProcessBuilder.start() will be >> terminated forcibly so we can rely on that. I don’t know how much it helps >> to wait for the process. If it wasn’t terminated, then we risk blocking >> forever here - still without having terminated the process. >> >>> I wonder whether JTReg kills the process tree on timeout - in case it does >>> using waitFor() would guarantee that there would be no zombies left. >>> Without using waitFor() and semantics of destroyForcibly() there might be >>> situations when non-functional stuck processes are left behind (not sure >>> how probable, however). >> >> JTreg currently has no process tree handling - there is work in progress to >> add it as it is clearly desirable. > > Ok. These are valid reasons for not using waitFor() - > http://cr.openjdk.java.net/~jbachorik/8059034/webrev.01 > > -JB- > >> >> /Staffan >> >> >>> >>> -JB- >>> >>>> >>>> /Staffan >>>> >>>> >>>> On 25 sep 2014, at 11:54, Jaroslav Bachorik <jaroslav.bacho...@oracle.com> >>>> wrote: >>>> >>>>> Please, review the following change to the JDK test library class >>>>> >>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8059034 >>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8059034/webrev.00 >>>>> >>>>> Currently, the ProcessTools.startProcess() might leave a dangling process >>>>> behind when a timeout or interrupt happens. The solution is to try and >>>>> forcibly terminate the forked process when this happens. >>>>> >>>>> Thanks, >>>>> >>>>> -JB-
