Hi Martin,
Thanks for the review.
On 4/11/2015 1:37 AM, Martin Buchholz wrote:
Thanks for the huge effort. I did a superficial review and it seems
pretty good. Of course, changing the Process good is high risk and
some things will probably need fixup later.
On Unix, you seem to be identifying ProcessHandles by their pid. What
are you going to do about the problem of misidentifying Unix processes
whose pid got recycled? If you spin up a thread to call waitpid as
soon as you create the ProcessHandle, the window is very short (and
OSes avoid recycling pids immediately) so that's pretty safe, but it
wastes a thread for each ProcessHandle! I don't have a good answer.
Maybe record the start time of the process, where available?
Getting the basic API and implementation in has taken too long, so I'm
looking to get it general agreement
and pushed and then I'd be happy to go back and include the start time
in ProcessHandle.
In the case of children() which already has to read /proc/pid/stat for
the parent,
the start time is inexpensively available also. The spec for equals(),
and compareTo is straightforward
without the start time. With the start time, either starttime has be
exposed in the API/spec or
be left 'implementation' dependent which is a poor spec.
Only in the case of allProcesses() would the overhead increase
significantly, it currently
does not need to open /proc/pid for every process.
+ } else if (siginfo.si_code == CLD_KILLED || siginfo.si_code ==
CLD_DUMPED) {
Using siginfo probably won't work under load because multiple signals
arriving at the same time are coalesced into one, at least on Linux.
Unix signals are sadly totally broken.
Siginfo is only used with waitid as an output argument
For the case of reaping the status, it uses waitpid (as Process did
before),
that seemed the least risky.
For the case of waiting for a process without reaping, that option is
only available
using waitid. I tried using waitid for both cases but on Mac, waitid
didn't seem to
be working correctly in all cases.
--
It seems you don't support sending arbitary signals, which is a little
sad.
I had not investigated any potential negative and unknowable effects of
being able to send arbitrary signals.
The implementation is easy to generalize, though it does not map to
Windows well/at all.
Would you propose it as an 'int' argument with no limitation? or what
limits?
I think it would be an additional method, the current
destory/destroyForcibly.
Thanks, Roger
On Thu, Apr 9, 2015 at 1:00 PM, Roger Riggs <roger.ri...@oracle.com
<mailto:roger.ri...@oracle.com>> wrote:
Please review the API and implementation of the Process API Updates
described inJEP 102
<https://bugs.openjdk.java.net/browse/JDK-8046092>. Please review
and comment by April 23rd.
The recommendation to make ProcessHandle an interface is included
allowing the new functions to be extended by Process subclasses.
The implementation covers all functions on Unix, Windows, Solaris,
and Mac OS X.
The API doc: http://cr.openjdk.java.net/~rriggs/ph-apidraft/
<http://cr.openjdk.java.net/%7Erriggs/ph-apidraft/>
The webrev: http://cr.openjdk.java.net/~rriggs/webrev-ph
<http://cr.openjdk.java.net/%7Erriggs/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.
Please review and comment, Roger
