JEP 102 Process Updates revised API draft

Tue Mar 10 09:17:37 UTC 2015

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.

Returning 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...

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 new API and we can mandate the implementations to 
support some sort of total-order Comparable even if getPid() is not 
supported. That reises 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.

Regards, Peter

> And 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.hegarty at oracle.com
>>> To: Roger.Riggs at oracle.com; jason_mehrens at hotmail.com
>>> CC: core-libs-dev at 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 if
>>>> getPid
>>>> throws UOE and without diluting the spec for getPid returning the 
>>>> native
>>>> os process identifier.
>>> Yes, that would work.
>>>
>>>> Defining the default for getPid() to return -1, might not have too big
>>>> an impact.
>>>> It would order the incompletely implemented Process subtypes before 
>>>> the
>>>> real ones
>>>> but the order is not usually significant except to be able to have a
>>>> predictable iteration order
>>>> or use TreeMap. Returning Long.MAX_VALUE as the default might be 
>>>> another
>>>> option.
>>> 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 throw
>>>>>> UOE.
>>>>> Ooh... I don't like this.
>>>>>
>>>>> Has any consideration been given to getPid returning -1, if 
>>>>> unknown or
>>>>> the default implementation? Or would this be any better?
>>>>>
>>>>> -Chris
>>>>>
>>>>>>
>>>>>>
>>>>>> Jason
>>>>>>
>>>>>>
>>>>>> ----------------------------------------
>>>>>>> Subject: Re: JEP 102 Process Updates revised API draft
>>>>>>> From: chris.hegarty at oracle.com
>>>>>>> Date: Fri, 6 Mar 2015 11:59:28 +0000
>>>>>>> To: Roger.Riggs at oracle.com
>>>>>>> CC: core-libs-dev at 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 
>>>>>>> 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?
>>>>>>>
>>>>>>> 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 is
>>>>>>> provided 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 not
>>>>>>> just 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 its
>>>>>>> existence. ( 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.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>
>