Hi Peter, On 3/10/2015 5:17 AM, Peter Levart wrote:
Correct, though if cached lazily, there might be an issue with its validity some indeterminate time later; but that's not worse than the case today without a timestamp.Hi,On 03/09/2015 07:46 PM, Roger Riggs wrote:Hi Jason,Comparing with startInstant is a possibility, assuming it is cached in the ProcessHandle and increases the cost of creating ProcessHandles since it needs to be parsed from /proc files.Unless it is obtained lazily and then cached. Internal logic of Process[Handle][Impl] doesn't need Comparable interface, right? Only client code might need it to order instances and identify duplicates.
That's always an option; though it will be harder for an application to cope with havingReturning 0 from ph1.compareTo(ph2) for two otherwise distinct system processes (in case getPid() is not supported) means that such objects can't be used as keys in TreeMap. If we can't support Comparable to be a total-order that differentiates objects based on some form of process identity, we might better not implement Comparable and let users use getPid() and/or make their own Comparators to support custom Process[Handle] implementations, etc...
to refer to comparators for custom implementations.
yes, for example, Instead of -1, Process.getPid() could assign a unique negative long.At least for Process objects that are guaranteed to be singletons per system process, there could be a tie-breaking integer allocated at construction time.
ProcessHandle is designed to not to have an external implementations (the constructor is package private). Process can be extended, exposing the issues Paul has raised. But yes, equals should reflect that two instances refer to the same system process (as far as can be determined from the OS).ProcessHandle is new API and we can mandate the implementations to support some sort of total-order Comparable even if getPid() is not supported. That raises the question: should we also mandate ProcessHandle.equals() to return true for ph1.equals(ph2) when and only when ph1 represents the same system process as ph2? Process objects already behave like that with default equals (since they are singletons per process). To be sure that every external implementation of ProcessHandle adheres to that specification, both compareTo() and equals()/hashCode() should be declared as abstract in ProcessHandle.
What this means is that different implementations of ProcessHandle won't be mutually comparable or equatable, but I don't see this as a problem if it is specified explicitly.
ok Thanks, Roger
Regards, PeterAnd it can help a long standing potential issue of pid's being re-used.ProcessHandle is only within a single host; there is no cross host invocation mechanism.Roger On 3/9/2015 12:49 PM, Jason Mehrens wrote:Anything but allowing UOE to escape compareTo sounds good.Apologies if I missed this in previous threads but, shouldn't ProcessHandle.compareTo compare hostname, startInstant, and then pid? Or assuming they are not comparable between hosts then just startInstant and pid.Jason ----------------------------------------Date: Mon, 9 Mar 2015 14:43:20 +0000 From: chris.hega...@oracle.com To: roger.ri...@oracle.com; jason_mehr...@hotmail.com CC: core-libs-dev@openjdk.java.net Subject: Re: JEP 102 Process Updates revised API draft On 09/03/15 14:28, Roger Riggs wrote:Hi,The problem could be isolated to compareTo by defining the ordering ifgetPidthrows UOE and without diluting the spec for getPid returning the nativeos process identifier.Yes, that would work.Defining the default for getPid() to return -1, might not have too bigan impact.It would order the incompletely implemented Process subtypes before thereal ones but the order is not usually significant except to be able to have a predictable iteration orderor use TreeMap. Returning Long.MAX_VALUE as the default might be anotheroption.That would probably be ok too, and then the UOE could be removed from Process.getPid() too, right? Which solves that small API wart. -Chris.Roger On 3/9/2015 6:10 AM, Chris Hegarty wrote:On 06/03/15 19:34, Jason Mehrens wrote:Hi Chris,Since getPid can throw UOE that means that compareTo could now throwUOE.Ooh... I don't like this.Has any consideration been given to getPid returning -1, if unknown orthe default implementation? Or would this be any better? -ChrisJason ----------------------------------------Subject: Re: JEP 102 Process Updates revised API draft From: chris.hega...@oracle.com Date: Fri, 6 Mar 2015 11:59:28 +0000 To: roger.ri...@oracle.com CC: core-libs-dev@openjdk.java.net Roger, I’ve taken a look at these changes in the sandbox ( JDK-8046092-branch ). Overall I welcome this addition. Some comments, most of which I stuffed into a webrev based on your branch, http://cr.openjdk.java.net/~chegar/process_comments/webrev/ 1) ProcessHandle.compareTo() can drop the ClassCastException Also, I think the comparison is the wrong way around. It should be compare(this, other), rather than compare(other, this), right? 2) I know there has been a lot of discussion about the use of CF, but I have a few more comments:a) Both onExist and onProcessExit are implemented to unconditionallythrow 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?b) The wording in the spec talks about async functions and actions. I think this may be not quite right. The intention is to support, as isprovided by CF, the ability to chain both sync and async tasks. [ I suggested some wording in the webrev ]c) Why the need for ISE if the process is the current process, and notjust return a CF that never completes? Do you consider this an error situation that you want to notify, or consistency with other parts of the API ? d) I wonder if onProcessExit should have a small API note, saying that it is preferred over onExit, when you have a Process. Or something to promote its use over onExit, or briefly explain itsexistence. ( I know why it is there, but it may appear as duplication )e) Maybe onProcessExit would benefit from an apiNote to indicate that it is essentially an alternative to waitFor() ? 3) Should ProcessHandle.getPid declare that it can throw IOE? Process.getPid declares UOE. 4) ProcessHandle.Info.user() ? owner() seems more appropriate, no? 5) The description of info() talks about fields, when it is an interface. I added some suggested rewording. Also, all methods now return references, so -1 can be removed. Similar for the Info class description. 6) There are a couple of @since 1.9 tags missing from Process supportsDestroyForcibly and onProcessExit That’s all for now. -Chris.
