JEP 102 Process Updates revised API draft

[Jason Mehrens]

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