RFR 9: 8077350 Process API Updates Implementation Review

[Roger Riggs]

Hi Thomas,

On 4/17/2015 4:22 AM, Thomas Stüfe wrote:
> Hi Roger,
>
> aside from the recycle-pid-question, one additional remark:
>
> in ProcessHandleImpl_unix.c, 
> Java_java_lang_ProcessHandleImpl_isAlive0, you call kill(pid, 0) for 
> the liveness check.
>
> If you have not the necessary permissions to do this call, this may 
> fail with EPERM. In this case, isAlive() will return false, but the 
> process exists, which strictly spoken is a lie.
>
> Caller may base actions on this, e.g. try to clean up resources for a 
> process he thinks is dead.
>
> We actually had this problem, and our version of isAlive() grew to be 
> more elaborate over time. Basically, we do:
>
> 1) first try kill(0). If EPERM:
> 2) try read /proc on OSes which have /proc.
> 3) do a system("ps").
>
> Alternativly, one may report EPERM to the caller - with an exception 
> or an extension of the return value - and leave it up to him what to do.
Good to have the experienced input.  I'd been trying to keep the code 
simple but it seems
more complexity is needed to be robust.  I'm not sure I'd go as far as 
invoking ps; it too might
fail for a number of reasons.  That would be a good case to throw an 
exception if the liveness
cannot be determined.

Thanks, Roger

>
> Kind Regards, Thomas
>
>
>
>
> On Thu, Apr 9, 2015 at 10:00 PM, Roger Riggs <Roger.Riggs at oracle.com 
> <mailto:Roger.Riggs at oracle.com>> wrote:
>
>     Please review the API and implementation of the Process API Updates
>     described inJEP 102
>     <https://bugs.openjdk.java.net/browse/JDK-8046092>. Please  review
>     and comment by April 23rd.
>
>     The recommendation to make ProcessHandle an interface is included
>     allowing the new functions to be extended by Process subclasses.
>     The implementation covers all functions on Unix, Windows, Solaris,
>     and Mac OS X.
>
>     The API doc: http://cr.openjdk.java.net/~rriggs/ph-apidraft/
>     <http://cr.openjdk.java.net/%7Erriggs/ph-apidraft/>
>
>     The webrev: http://cr.openjdk.java.net/~rriggs/webrev-ph
>     <http://cr.openjdk.java.net/%7Erriggs/webrev-ph>
>
>     Issue: JDK-8077350
>     <https://bugs.openjdk.java.net/browse/JDK-8077350> Process API
>     Updates Implementation
>
>     The code is in the jdk9 sandbox on branch JDK-8046092-branch.
>
>     Please review and comment, Roger
>
>