RFR 8059034: ProcessTools.startProcess() might leak processes

2014-09-25 Thread Jaroslav Bachorik
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

Re: RFR 8059034: ProcessTools.startProcess() might leak processes

2014-09-25 Thread Staffan Larsen
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. /Staffan On 25

Re: RFR 8059034: ProcessTools.startProcess() might leak processes

2014-09-25 Thread Jaroslav Bachorik
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

Re: RFR 8059034: ProcessTools.startProcess() might leak processes

2014-09-25 Thread Mikael Auno
On 2014-09-25 12:24, Jaroslav Bachorik 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,

Re: RFR 8059034: ProcessTools.startProcess() might leak processes

2014-09-25 Thread Staffan Larsen
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

Re: RFR 8059034: ProcessTools.startProcess() might leak processes

2014-09-25 Thread Jaroslav Bachorik
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

Re: RFR 8059034: ProcessTools.startProcess() might leak processes

2014-09-25 Thread Staffan Larsen
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

Re: RFR 8059034: ProcessTools.startProcess() might leak processes

2014-09-25 Thread roger riggs
Hi, The spec for destroyForcibly goes on to say that ProcessBuilder implements destroyForcibly correctly. Invoking this method on Process objects returned by ProcessBuilder.start() and Runtime.exec(java.lang.String) will forcibly terminate the process. Roger On 9/25/2014 6:24 AM,