Hi Chris,
Since getPid can throw UOE that means that compareTo could now throw UOE. Jason ---------------------------------------- > Subject: Re: JEP 102 Process Updates revised API draft > From: chris.hega...@oracle.com > Date: Fri, 6 Mar 2015 11:59:28 +0000 > To: roger.ri...@oracle.com > CC: core-libs-dev@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. > > >
