JEP 102 Process Updates revised API draft

[Peter Levart]

On 03/10/2015 10:50 AM, Paul Sandoz wrote:
> On Mar 6, 2015, at 7:58 PM, Roger Riggs <Roger.Riggs at Oracle.com> wrote:
>>> 2) I know there has been a lot of discussion about the use of CF,
>>>      but I have a few more comments:
>>>           a) Both onExit and onProcessExit are implemented to unconditionally
>>>          throw UOE. Is the intention to make the implementation of these
>>>          methods optional? If so, then UOE should be documented, If not,
>>>          then I think they can be abstract, right?
>> There are existing non-jdk implementations of Process and it would be source
>> incompatible if default implementations were not supplied.  So, not abstract.
>>
>> Since the behavior applies to Process, the note in the class documentation
>> defines the behavior.
>> "Methods of {@code Process} that are not otherwise specified or overridden throw
>> {@link java.lang.UnsupportedOperationException}."
>>
>> More direct documentation could be provided in the override of onExit
>> but that would raise the visibility to ordinary developers who should use
>> onProcessExit that returns the correctly typed CF and not onExit.
>> The note should be sufficient for implementers without distracting the ordinary developer.
>>
>> Perhaps, a description similar to that used in destroy(), the behavior of Processes
>> returned from ProcessBuilder can be more tightly specified. Alternatively,
>> ProcessBuilder could specify the behavior of Process instances it returns, at present,
>> the behavior of the Process instances is split between the two classes.
>>
> Throwing USO for default implementations is a warning sign to me that something does not fit quite right. It feels like we are giving up :-)

I concur. I think current ProcessHandle is a mix of specification and 
partial implementation that does not help in any way to hypothetical 
external implementations. It effectively disables any consistent 
alternative implementations. Take for example the following method in 
ProcessHandle:

     public final ProcessHandle parent() {
         return ProcessHandleImpl.parent(getPid());
     }

There's no way an alternative implementation of ProcessHandle could be 
created that returned alternative implementations using method parent(). 
What if alternative implementation doesn't support getPid()?

I think ProcessHandle should be mostly a specification and only have 
abstract methods. Only a method that does not depend on implementation 
could be concrete. I think it could be an interface. A package-private 
constructor does not help as there is a Process class that has a public 
constructor already, so anyone can create arbitrary instances of 
arbitrary subclass of Process which is-a ProcessHandle...

>
> I now see Process.getPid was previously added and it throws USO by default. So we might be stuck due to the tricky nature of this area.

Unfortunately. Default methods (in publicly extendable abstract classes 
and interfaces) can not provide new functionality. They can just re-pack 
existing functionality. getPid() is new functionality and so are all new 
methods of ProcessHandle that Process inherits from. So we can not just 
simply delegate to our implementation which might be incompatible 
(unsupported platform).

>
> Any sub-type of Process that does not override getPid will essentially result in that USO being propagated to many ProcessHandle methods that depend on the PID (parent, children, allChildren, info, compareTo). That effectively renders ProcessHandle almost useless for sub-types outside of our control that that not been updated.

So here's some changes I propose to the tip of JDK-8046092-branch that 
hopefully fix these issues:

http://cr.openjdk.java.net/~plevart/jdk9-sandbox/JDK-8046092-branch/webrev.02/

The changes are:
- Process.getPid() is back to throwing UOE if unsupported
- ProcessHandle is all-abstract interface (only allChildren() is a 
default method, since it is just a utility independent of implementation)
- ProcessHandle.compareTo() specifies that only same-implementations of 
ProcessHandle(s) are mutually comparable otherwise ClassCastException 
must be thrown
- ProcessHandleImpl.onExit() and ProcessImpl.onProcessExit() propagate 
error from waitForProcessExit0 native method as exceptional completion 
of CompletableFuture. exitValue is set to -1 on UNIX in this case 
(instead of 0). What's the exitStatus on Windows in this case? This area 
needs more work, I think.
- Some tweaks to javadocs in a few places

>
> Is it common to sub-type Process? If so then perhaps Process should not extend ProcessHandle and instead there should be a way to view a Process as a ProcessHandle, whose default implementation is:
>
>    return ProcessHandle.of(getPid()); // throws USO if getPid does
>
> (java.lang.ProcessImpl could implement Processhandle thereby the implementation is "return this".)
>
> Thus once you have a ProcessHandle instance (from whatever source) it's possible to operate on it without fear of a USO.
>
>
> A possible default implementation for Process.onProcessExit could be:
>
>    return CF.supplyAsync(() -> { this.waitFor(); return p}, cachedThreadPool);

It could be, but this would encourage external implementations to not 
override it and this implementation is not very efficient in terms of 
thread stack sizes. We could use a default implementation that is 
similar to Windows' ProcessImpl - only use waitFor() instead of 
ProcessHandleImpl.waitForProcessExit0() native method, but on UNIX 
waitFor() *is* implemented on top of this implementation, so I think the 
best is to force the implementor to do it optimally from the ground-up. 
Default methods should really be reserved for something very trivial 
that is not only implementation independent, but also equally efficient 
regardless of underlying implementation.

Regards, Peter

>
> Paul.