Hi Peter,

If delegating to another Process and not needing to return the CF<subtype>
then the overridden implementation might be:
        public CompletableFuture<Process> onExit() {
return delegate.onExit();
        }

The .thenApply(ph -> this) would be needed only for the return value to be CF<subtype>.
The explanation can get lengthy if the best-practice options are described
and mentioning toHandle() reintroduces the possibility of UOE.

     * @implNote
     * The default implementation of this method employs a thread from
     * {@link java.util.concurrent.ForkJoinPool#commonPool() common pool}
     * to {@link #waitFor() wait} for process exit, which might consume a lot
     * of memory for thread stacks if large number of processes are waited for
     * concurrently.<p>
     * External implementations are advised to override this method and provide
     * more efficient implementation, for example, if the implementation also
     * provides {@link #toHandle()} method, it can simply do the following:
     * <pre>{@code
     *    public CompletableFuture<Process> onExit() {
     *       return delegate.onExit();
     *    }
     * }</pre>
     * ...which in case of implementation of the delegate is used.


A more general class level note about delegating each of the methods of the
underlying process might be sufficient but might be overlooked.

Thanks, Roger

On 3/13/2015 3:35 AM, Peter Levart wrote:
Hi Roger,

Now that the method has a non-throwing default implementation, what do you think of the following implementation note for Process.onExit():

      * @implNote
      * The default implementation of this method employs a thread from
      * {@link java.util.concurrent.ForkJoinPool#commonPool() common pool}
      * to {@link #waitFor() wait} for process exit, which might consume a lot
      * of memory for thread stacks if large number of processes are waited for
      * concurrently.<p>
      * External implementations are advised to override this method and provide
      * more efficient implementation, for example, if the implementation also
      * provides {@link #toHandle()} method, it can simply do the following:
      * <pre>{@code
      *    public CompletableFuture<Process> onExit() {
      *       return toHandle().onExit().thenApply(ph -> this);
      *    }
      * }</pre>
      * ...which in case of internal implementation of ProcessHandle
      * ({@link java.lang.ProcessHandle#of(long)}}), employs a more efficient
      * mechanism for waiting on process exit.



Regards, Peter


On 03/12/2015 11:33 PM, Peter Levart wrote:


On 03/12/2015 09:41 PM, Roger Riggs wrote:
Hi Peter,

Introducing a public Process subtype would not be a binary compatible change;
the return type of ProcessBuilder.start can not be narrowed.
As you surmised, a different start method would be needed on ProcessBuilder. Since ProcessBuilder is the preferred mechanism to created processes, I would
leave Runtime.exec alone to avoid a proliferation of similar methods.

If ProcessHandle were an interface, Process would still have the conflict over the return type of onExit() since CompletableFuture<Process> is not type compatible with CF<ProcessHandle>. So not quite the winning combination to enable polymorphism.

Roger

That's right. The delegation approach with unrelated Process / ProcessHandle seems to be the most clean from API perspective.

Regards, Peter



On 3/12/2015 10:38 AM, Peter Levart wrote:
On 03/12/2015 02:39 PM, Roger Riggs wrote:
Hi,

Just a thought, it might be useful to introduce a public subtype of Process that is returned from ProcessBuilder for which the guarantees about behavior could be tighter (no UOEs). It would also provide a place to document the behaviors
now spread across ProcessBuilder and Process.

$.02, Roger


That was my thinking too today. A Process2 or XProcess? If ProcessHandle was an interface this subtype could implement it (back to square one). I think it could be an interface if Process2 was not publicly extendable (package-protected constructor) as it would not represent part of extensible API.

Does that mean that we would need additional methods ProcessBuilder.start2() / Runtime.exec2() too?

Peter





Reply via email to