RFR 9: 8077350 Process API Updates Implementation Review

[Peter Levart]

Hi Roger,

The new API using Optional(s) looks fine. In particular for the 
ProcessHandle returning methods. They now either return 
Stream<ProcessHandle> or Optional<ProcessHandle>.

At some point in the development of this API, the implementation 
introduced the AsyncExecutor to execute synchronous continuations of the 
onExit() returned CompletableFuture(s). What was the main motivation for 
this given that:
- previously, ForkJoinPoll.commonPool() was used instead that by default 
possesses some similar characteristics (Innocuous threads when 
SecurityManager is active)
- this AsyncExecutor is only effective for 1st "wave" of synchronous 
continuations. Asynchronous continuations and synchronous continuations 
following them will still use ForkJoinPoll.commonPool()

Would an alternative be to define two overloaded onExit() methods in the 
style of CompletableFuture itself?

     CompletableFuture<ProcessHandle> onExit();
     CompletableFuture<ProcessHandle> onExit(Executor executor);

...and give the user a chance to supply it's own Executor if the default 
ForkJoinPoll.commonPool() does not fit?

Is there expectation that ForkJoinPoll.commonPool() will not fit in the 
common case?

Regards, Peter

On 05/13/2015 04:16 PM, Roger Riggs wrote:
> Hi,
>
> Are there any comments about the use of java.util.Optional in the 
> ProcessHandle API?
> Or a review of the changes?
>
> Thanks, Roger
>
>
> On 5/11/2015 11:49 AM, Roger Riggs wrote:
>> Please review clarifications and updates to the proposed Precess API.
>>
>> A few loose ends in the ProcessHandle API were identified.
>>
>> 1) The ProcessHandle.parent() method currently returns null if the 
>> parent cannot
>> be determined and the ProcessHandle.of(PID) method returns null if 
>> the PID does not exist.
>> It has been suggested to return an Optional<ProcessHandle> to make
>> these methods more flexible and allow a fluent style and work better 
>> with streams.
>>
>> 2) The behavior of Processhandle.destroy and destroyForcibly are 
>> different
>> than Process.destroy and destroyForcibly.  Those functions always 
>> succeed because
>> they are children of the spawning process.
>>
>> In contrast, ProcessHandle.destroy and destroyForcible are requests to
>> destroy the process and may not succeed due to operating system 
>> restrictions such
>> as the process not being a child or not having enough privilege.
>> The description of the methods needs to be clarified that it is a 
>> request to destroy
>> and it may not succeed, In that case the destroy and destroyForcibly 
>> methods
>> should indicate that the request was not successful.  In particular, 
>> the caller
>> may not want to wait for the process to terminate (its not going to).
>>
>> The proposed update is to return an Optional<ProcessHandle> .
>> It can be streamed and can take advantage of the conditional 
>> operations on the Optional.
>>
>> 3) Using Optional is also attractive for the return values of the 
>> information
>> about a ProcessHandles, since not all values are available from every 
>> OS.
>> The returns values of Info.command, arguments, startInstant, 
>> totalDuration, and user
>> are proposed to be updated to return Optional<x>.
>> It allows for more compact code and fewer explicit checks for null.
>>
>> Please review and comment:
>>
>> Webrev:
>>    http://cr.openjdk.java.net/~rriggs/webrev-ph/
>>
>> javadoc:
>>    http://cr.openjdk.java.net/~rriggs/ph-apidraft/
>>
>> Diffs of the spec/javadoc from previous draft:
>> http://cr.openjdk.java.net/~rriggs/ph-diffs-2015-05-11/overview-summary.html 
>>
>>
>> Thanks, Roger
>>
>>
>>
>