JEP 102 Process Updates revised API draft

[Roger Riggs]

Hi Peter,

On 3/10/2015 5:17 AM, Peter Levart wrote:
> 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.
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.
>
> 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...
That's always an option; though it will be harder for an application to 
cope with having
to refer to comparators for custom implementations.
>
> 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.
yes, for example, Instead of -1, Process.getPid() could assign a unique 
negative long.
>
> 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.
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).
>
> 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, 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.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>
>>
>