RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

[Roger Riggs]

Hi Paul,

On 4/20/2015 9:01 AM, Paul Sandoz wrote:
> Hi Roger,
>
> I am not sure you have the @implSpec/@implNote quite correct on the new methods of Process.
>
> For example, for Process.toHandle i would expect something like:
>
>    ...
>
>    @implSpec
>    This implementation throws an instance of UnsupportedOperationException and
>    performs no other action.  Sub-types should override this method, ensuring that
>    calling methods (getPid etc.) of this class, that are not overridden, operate on the
>    returned ProcessHandle.
>
> The @implSpec should refer to the implementation in Process itself, and the @implNote cannot be used for any normative statements.
Thanks for the reminder and suggested text.  I updated the @implSpec 
clauses.

Some @implNotes describe the JDK implementation and some developers will
rely on the implementation specifics and to some degree define the 
expected behaviors.
>
>
> The document for Process.getPid (and similarly those methods depending on toHandle) could then be:
>
>    ...
>    @implSpec
>    This implementation returns a process id as follows:
>
>      toHandle().getPid();
>
>
> In this respect is there a need to say anything about the behaviour of a Process created by ProcessBuilder?
The ProcessBuilder produced subclasses of Process implement the spec
so no additional description is needed.
>
> It might be useful to have some general guidance for sub-types on the class doc of Process e.g. saying they only need to override toHandle but may override some or all dependent methods as appropriate.
It does not add much but I added a paragraph to the Process class javadoc.
There are not many subclasses of Process outside the JDK.

The webrev[1] and javadoc[2] have been updated in place.

Thanks, Roger

[1] http://cr.openjdk.java.net/~rriggs/webrev-ph
[2] http://cr.openjdk.java.net/~rriggs/ph-apidraft/

>
> Paul.
>
> On Apr 17, 2015, at 9:12 PM, Roger Riggs <Roger.Riggs at Oracle.com> wrote:
>
>> The webrev for ProcessAPI updates has been updated to reflect recent comments.
>> Please  review and comment by April 23rd.
>>
>> The updates include:
>> - Renaming Process/ProcessHandle supportsDestroyForcibly to supportsNormalTermination
>>    and updating related descriptions
>> - ProcessHandle.destroy/destroyForcible descriptions have more consistent descriptions
>> - ProcessHandle.destroy now returns ProcessHandle to enable fluent use.
>> - Corrected description of default implementation ProcessHandle.onExit
>>
>> The API doc: http://cr.openjdk.java.net/~rriggs/ph-apidraft/
>>
>> The webrev: http://cr.openjdk.java.net/~rriggs/webrev-ph
>>
>> Issue: JDK-8077350 <https://bugs.openjdk.java.net/browse/JDK-8077350> Process API Updates Implementation
>>
>> The code is in the jdk9 sandbox on branch JDK-8046092-branch.
>>
>> Two issues raised have been filed to be fixed after the current commit:
>> - JDK-8078099 <https://bugs.openjdk.java.net/browse/JDK-8078099> (process) ProcessHandle should uniquely identify processes
>> - JDK-8078108 <https://bugs.openjdk.java.net/browse/JDK-8078108> (process) ProcessHandle.isAlive should be robust