RFR 9: 8176272 : (process) ProcessHandle::onExit fails to wait for non-child process

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR 9: 8176272 : (process) ProcessHandle::onExit fails to wait for non-child process

roger riggs
Please review a change to the implementation of ProcessHandle.onExit to
correctly handle waiting for any process to terminate and a
corresponding new test.
On Linux, waitpid only waits for direct children; waiting for non-children
now polls for termination.

Please review and comment:

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-processhandle-onexit-8176272/index.html

Issue:
   https://bugs.openjdk.java.net/browse/JDK-8176272

Thanks, Roger



Reply | Threaded
Open this post in threaded view
|

Re: RFR 9: 8176272 : (process) ProcessHandle::onExit fails to wait for non-child process

Thomas Stüfe-2
Hi Roger,

when using isAlive0, would it may make sense to - on the first invocation -
remember the process start time and on subsequent invocations to check this
time against the new return value? That way you could check for process
identity in the case of recycled process ids.

Kind Regards,Thomas

On Tue, Mar 14, 2017 at 9:36 PM, Roger Riggs <[hidden email]> wrote:

> Please review a change to the implementation of ProcessHandle.onExit to
> correctly handle waiting for any process to terminate and a corresponding
> new test.
> On Linux, waitpid only waits for direct children; waiting for non-children
> now polls for termination.
>
> Please review and comment:
>
> Webrev:
> http://cr.openjdk.java.net/~rriggs/webrev-processhandle-onex
> it-8176272/index.html
>
> Issue:
>   https://bugs.openjdk.java.net/browse/JDK-8176272
>
> Thanks, Roger
>
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 9: 8176272 : (process) ProcessHandle::onExit fails to wait for non-child process

roger riggs
Hi Thomas,

Good idea.  Though it is unlikely that the pid would be re-used between
the checks of isAlive
but that will remove any window.

Updated webrev:
http://cr.openjdk.java.net/~rriggs/webrev-processhandle-onexit-8176272/index.html

Thanks, Roger


On 3/15/2017 4:19 AM, Thomas Stüfe wrote:

> Hi Roger,
>
> when using isAlive0, would it may make sense to - on the first
> invocation - remember the process start time and on subsequent
> invocations to check this time against the new return value? That way
> you could check for process identity in the case of recycled process ids.
>
> Kind Regards,Thomas
>
> On Tue, Mar 14, 2017 at 9:36 PM, Roger Riggs <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Please review a change to the implementation of
>     ProcessHandle.onExit to
>     correctly handle waiting for any process to terminate and a
>     corresponding new test.
>     On Linux, waitpid only waits for direct children; waiting for
>     non-children
>     now polls for termination.
>
>     Please review and comment:
>
>     Webrev:
>     http://cr.openjdk.java.net/~rriggs/webrev-processhandle-onexit-8176272/index.html
>     <http://cr.openjdk.java.net/%7Erriggs/webrev-processhandle-onexit-8176272/index.html>
>
>     Issue:
>     https://bugs.openjdk.java.net/browse/JDK-8176272
>     <https://bugs.openjdk.java.net/browse/JDK-8176272>
>
>     Thanks, Roger
>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 9: 8176272 : (process) ProcessHandle::onExit fails to wait for non-child process

Chris Hegarty

> On 15 Mar 2017, at 18:53, Roger Riggs <[hidden email]> wrote:
>
> Hi Thomas,
>
> Good idea.  Though it is unlikely that the pid would be re-used between the checks of isAlive
> but that will remove any window.
>
> Updated webrev:
> http://cr.openjdk.java.net/~rriggs/webrev-processhandle-onexit-8176272/index.html

This looks good to me Roger.

Trivially, I think I would have defined an
   @Native static final int NOT_A_CHILD = -2;
 , but that is just a minor cleanup/style comment.

-Chris.
Reply | Threaded
Open this post in threaded view
|

Re: RFR 9: 8176272 : (process) ProcessHandle::onExit fails to wait for non-child process

Thomas Stüfe-2
In reply to this post by roger riggs
Thanks Roger. This looks good to me now.

Thomas

On Wed, Mar 15, 2017 at 7:53 PM, Roger Riggs <[hidden email]> wrote:

> Hi Thomas,
>
> Good idea.  Though it is unlikely that the pid would be re-used between
> the checks of isAlive
> but that will remove any window.
>
> Updated webrev:
>   http://cr.openjdk.java.net/~rriggs/webrev-processhandle-
> onexit-8176272/index.html
>
> Thanks, Roger
>
>
>
> On 3/15/2017 4:19 AM, Thomas Stüfe wrote:
>
> Hi Roger,
>
> when using isAlive0, would it may make sense to - on the first invocation
> - remember the process start time and on subsequent invocations to check
> this time against the new return value? That way you could check for
> process identity in the case of recycled process ids.
>
> Kind Regards,Thomas
>
> On Tue, Mar 14, 2017 at 9:36 PM, Roger Riggs <[hidden email]>
> wrote:
>
>> Please review a change to the implementation of ProcessHandle.onExit to
>> correctly handle waiting for any process to terminate and a corresponding
>> new test.
>> On Linux, waitpid only waits for direct children; waiting for non-children
>> now polls for termination.
>>
>> Please review and comment:
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rriggs/webrev-processhandle-onex
>> it-8176272/index.html
>>
>> Issue:
>>   https://bugs.openjdk.java.net/browse/JDK-8176272
>>
>> Thanks, Roger
>>
>>
>>
>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 9: 8176272 : (process) ProcessHandle::onExit fails to wait for non-child process

roger riggs
Hi Thomas, Chris.

I updated the webrev with Chris's suggestion and will push after tests
complete.
http://cr.openjdk.java.net/~rriggs/webrev-processhandle-onexit-8176272/index.html 
<http://cr.openjdk.java.net/%7Erriggs/webrev-processhandle-onexit-8176272/index.html>

Thanks for the review and comments, Roger


On 3/16/2017 10:00 AM, Thomas Stüfe wrote:

> Thanks Roger. This looks good to me now.
>
> Thomas
>
> On Wed, Mar 15, 2017 at 7:53 PM, Roger Riggs <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi Thomas,
>
>     Good idea.  Though it is unlikely that the pid would be re-used
>     between the checks of isAlive
>     but that will remove any window.
>
>     Updated webrev:
>     http://cr.openjdk.java.net/~rriggs/webrev-processhandle-onexit-8176272/index.html
>     <http://cr.openjdk.java.net/%7Erriggs/webrev-processhandle-onexit-8176272/index.html>
>
>     Thanks, Roger
>
>
>
>     On 3/15/2017 4:19 AM, Thomas Stüfe wrote:
>>     Hi Roger,
>>
>>     when using isAlive0, would it may make sense to - on the first
>>     invocation - remember the process start time and on subsequent
>>     invocations to check this time against the new return value? That
>>     way you could check for process identity in the case of recycled
>>     process ids.
>>
>>     Kind Regards,Thomas
>>
>>     On Tue, Mar 14, 2017 at 9:36 PM, Roger Riggs
>>     <[hidden email] <mailto:[hidden email]>> wrote:
>>
>>         Please review a change to the implementation of
>>         ProcessHandle.onExit to
>>         correctly handle waiting for any process to terminate and a
>>         corresponding new test.
>>         On Linux, waitpid only waits for direct children; waiting for
>>         non-children
>>         now polls for termination.
>>
>>         Please review and comment:
>>
>>         Webrev:
>>         http://cr.openjdk.java.net/~rriggs/webrev-processhandle-onexit-8176272/index.html
>>         <http://cr.openjdk.java.net/%7Erriggs/webrev-processhandle-onexit-8176272/index.html>
>>
>>         Issue:
>>         https://bugs.openjdk.java.net/browse/JDK-8176272
>>         <https://bugs.openjdk.java.net/browse/JDK-8176272>
>>
>>         Thanks, Roger
>>
>>
>>
>>
>
>