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

[Roger Riggs]

Hi Paul,

There are statements in Process about the specified behavior of Processes
created by ProcessBuilder.  That's why I included them in the @implSpec 
clause.
If @implSpec is only for the specifics of the method itself then where
should the behavior of ProcessBuilder created instances be specified?

Thanks, Roger



On 4/20/2015 12:33 PM, Paul Sandoz wrote:
> On Apr 20, 2015, at 5:49 PM, Roger Riggs <Roger.Riggs at Oracle.com> wrote:
>
>> 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.
>>
> I spotted another one here:
>
>   281     /**
>   282      * Returns {@code true} if the implementation of {@link #destroy} is to
>   283      * normally terminate the process,
>   284      * Returns {@code false} if the implementation of {@code destroy}
>   285      * forcibly and immediately terminates the process.
>   286      *
>   287      * @implSpec
>   288      * Processes returned from ProcessBuilder support this operation to
>   289      * return true or false depending on the platform implementation.
>   290      *
>   291      * @return {@code true} if the implementation of {@link #destroy} is to
>   292      *         normally terminate the process;
>   293      *         otherwise, {@link #destroy} forcibly terminates the process
>   294      * @throws UnsupportedOperationException if the Process implementation
>   295      *         does not support this operation
>   296      * @since 1.9
>   297      */
>   298     public boolean supportsNormalTermination() {
>   299         throw new UnsupportedOperationException("supportsNormalTermination not supported for " + this.getClass().toString());
>   300     }
>
>
> The docs of Process.onExit might also require some tweaks. I dunno how much wiggle room there is with the current implementation, perhaps very little?
>
>
>   415     /**
>   416      * Returns a ProcessHandle for the Process.
>   417      *
>   418      * @implSpec
>   419      * This implementation throws an instance of UnsupportedOperationException
>   420      * and performs no other action.
> ...
>   446      * @implSpec
>   447      * The implementation of this method returns information about the process as:
>   448      * {@link #toHandle toHandle().info()}.
>
> Best to be consistent with either "This implementation ..." or "The implementation of this method ..." rather than a mix. I prefer the former as it is more concise. Up to you.
>
>
>> Some @implNotes describe the JDK implementation and some developers will
>> rely on the implementation specifics and to some degree define the expected behaviors.
> One way of thinking about this is that the developers writing the TCK tests will pay close attention to @implSpec and need not do so for @implNote.
>
>
>>>
>>> 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.
>>
> Ok.
>
> Paul.
>
>> 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/