JEP 102 Process Review

[Chris Hegarty]

Roger,

I think the API is looking much better now ( just a few comments below on small specific issues ), so I’ve taken a pass over the implementation changes in the sandbox. Here are some comments:

1) Some operations on ProcessHandle require RuntimePermission
    "manageProcess", but I don't see this specified in
    java.lang.RuntimePermission ( there is a table of target runtime
    permission names ). I think you just need to add it.

2) Can ProcessImpl.waitForProcessExit and its native counterpart be
    removed? ( since its function is now performed through ProcessHandleImpl )

3) "process reaper”, maybe
    “Process-Reaper-“ + monotonically increasing ID ( similar to
    ForkJoin worker threads, or other ). But I do accept that this
    is the existing name for the reaper threads.

4) Should Info.toString() print something if all fields are null,
    or -1.

5) Could the fields for Info be private final, and use a separate
    private holder for retrieving the information from native?
    Seems desirable for them to be final.

6) Process has s number of @implXXX tags, they typically appear
    before the @return in usage I’ve seen in the JDK.

7) Should ProcessHandleImpl.toString include Info, and/or other
    information about the handle? 

Subjective/Minor:

1) There are number of commented out fprintfs in native code
    that should be removed.

2) You may need to take a pass over the copyright year range.
    I understand that some of these files were created in 2014, but
    I suspect that they should all at least include 2015.

3) OnExitTest & InfoTest include the “Classpath” exception, when
    they should not.

4) There is some duplication of javadoc in implementation
    that could be removed, e.g:
       /**
        * Returns the process ID as a decimal String.
        * @return the process ID as a decimal String
        */

    My preference is to not repeat the public API doc in
    implementation classes, but I accept that this is sometimes
    done for in-place readability.

5) There are number of lines that are greater than 80 
    characters. These can be difficult for future maintenance
    and reviews ( webrev side-by-side diffs ).

I need to spend a little more time on the native code, but the tests pass so that is a good start!

-Chris.