Hi Peter,
On 3/10/2015 2:57 PM, Peter Levart wrote:
Hi Roger,
Thanks for bearing with me...
And me, as I find the route among the language, type system,
compatibility issues,
and os intricacies.
On 03/10/2015 06:27 PM, Roger Riggs wrote:
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());
}
ProcessHandle is not intended to be *externally* extended; as an
interface or abstract class
But Process is, and Process inherits methods from ProcessHandle which
means that you are limiting external implementations of Process
forever to status-quo functionality. Why not let the implementor of
external Process implementation decide whether it needs or wants to
implement this new functionality or not. Are you afraid that
implementations will emerge, but will not be of enough quality because
of impedance missmatch between the API and the target platform? Or are
you afraid that it will be harder to support new additions to this API
if there are external implementations created?
I'm reluctant because we're having to speculate about the details of
what might be
with no one to speak directly to any particular target OS issues and not
much evidence that it is needed.
it can not make *any* guarantees about the implementation.
No caller can depend on any of the behavior based on the compile time
type.
With the changes proposed, the callers using ProcessHandle must be
prepared to handle UOE
in addition to dealing with the normal absent or missing information
from the OS.
So let's say the caller is on a platform that is not supported by JDK
internal ProcessHandle implementation. What guarantees does
ProcessHandle implementation give him? It think he's in better
position if he must cope with differences than he is if he can't use
the API at all.
Perhaps
UOE from getPid() is just a form of absent information. It could be -1.
This choice goes to the design of the caller; is it structured to
handle exceptions
or willing to check the validity of the values.
Many have complained that code to handle exceptions makes less code
unreadable
and should be avoided.
We should not open up another hole like Process without fully
understanding
the use cases and the requirements on extensibility. There may be a
middle ground
that leaves the door open but not to start with.
The hole is already open. Some external implementation of Process can
already attempt to implement all the ProcessHandle instance API merely
by subclassing the Process. Package-private constructor on
ProcessHandle will not stop that. Final methods will though.
The argument is frequently made about making the surface area of the API
larger
without taking into account who pays the cost and who gets the benefit.
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 have not considered supporting alternate implementations a goal but
not breaking existing ones is essential.
Well, I don't see anything existing is broken if ProcessHandle doesn't
impose final methods.
True for existing API; but then the salient discussion is about the
semantics of the new functions
and how they can be expected to adhere to the contract if extensibility
is supported.
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...
Most of the behavior of ProcessHandle (as is Process) *is*
implementation specific due to the OS dependencies.
When every function bottoms out in OS behavior, the layers above are
dependent too.
So this is a question of: "Is ProcessHandle API suitable for other
implementations or is it just for supported UNIX/Windows platforms",
right?
Because technically it could be exposed as externally implementable
with no backwards compatibility problems when 1st introduced. There
might be problems later when extending it.
That's the risk. Can it be eliminated or avoided... Is it a necessary
unknown?
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).
Yes, but to support source level compatibility; some implementation
is needed; though it cannot provide the full functionality.
There's no source-level compatibility issue with ProcessHandle as it
is new class or interface. Just with Process. And even with my
changes, the source-level compatibility is maintained. Can you spot
anything that is not source-compatible or does not behave like it used
to? It actually behaves exactly like it behaves with your latest
implementation when default Process.getPid() throws UOE. The only
difference is that ProcessHandle is made extensible.
Just the aforementioned risk to maintenance cost
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)
I considered 'abstract' in the PH API to be a distraction unless the
API was intended to be extensible.
Well in that case, yes. But why is it ProcessHandle then split between
ProcessHandle and ProcessHandleImpl? Isn't that a distraction too?
It kept the bulk of the implementation details out of the API and made
them available
to the ProcessImpls. I'm still working on the organization of the
implementation; there should
be a clean split between ProcessImpl's that spawn processes and
ProcessHandleImpl for the other functions.
- ProcessHandle.compareTo() specifies that only same-implementations
of ProcessHandle(s) are mutually comparable otherwise
ClassCastException must be thrown
Yes, avoids the issue with UOE.
- 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.
The exceptional case was removed because the waitForExit does not
throw an exception.
The only exception from Process.waitFor is InterruptedException and
since there might
be spurious exceptions it should be retried.
When waitForProcessExit0 returns the process is gone; and the CF can
be completed.
So there's no error condition from waitForProcessExit0() then. Any
return *is* exitValue(). I haven't checked the native implementations
and just relied on comments in Java code.
There are a variety of OS level possible error conditions but either the
process still exists
in which case it should still wait; or it is done and the exitValue (or
not-available value)
is returned.
- 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.
Without external implementations, the code moved up from ProcessImpl
and ProcessHandleImpl classes
to the abstract class to avoid duplication in the subclasses.
ProcesshandleImpl and Windows ProcessImpl can have same code, but UNIX
ProcessImpl must additionally synchronize using waitFor() since on
UNIX, exitValue() result is published by the continuation of
completion() which can execute concurrently with asynchronous
continuation of same completion() that completes user-visible
CompletableFuture. There's no harm if synchronization using waitFor()
is used in ProcesshandleImpl and Windows ProcessImpl too, but that's
unneeded overhead.
yes, on Unix, it provides synchronization between the CF completions.
On Windows, it is redundant since by the time that code is executed, the
process is already known to be done.
Thanks, Roger
