JEP 102 Process Updates revised API draft

Chris Hegarty chris.hegarty at oracle.com
Fri Mar 6 11:59:28 UTC 2015


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.






More information about the core-libs-dev mailing list