RFR 8059034: ProcessTools.startProcess() might leak processes

Staffan Larsen staffan.larsen at oracle.com
Thu Sep 25 10:45:23 UTC 2014


Looks good!

No need for the assignment to p, though.

/Staffan


On 25 sep 2014, at 12:39, Jaroslav Bachorik <jaroslav.bachorik at oracle.com> wrote:

> On 09/25/2014 12:33 PM, Staffan Larsen wrote:
>> 
>> On 25 sep 2014, at 12:24, Jaroslav Bachorik <jaroslav.bachorik at 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.bachorik at 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-



More information about the jdk9-dev mailing list