Thanks Thomas,
As per Alan's mail, we're going to deal with this as a separate issue
after we've discussed it.
-Rob
On 25/06/12 10:52, Alan Bateman wrote:
On 25/06/2012 09:56, Thomas Stüfe wrote:
Hi, Rob,
src/solaris/native/java/lang/UNIXProcess_md.c:
You never check the return code
Hi, Rob,
src/solaris/native/java/lang/UNIXProcess_md.c:
You never check the return code of kill(2). kill() may fail if you
have no permission to kill the process (EPERM), or if the process id
is invalid, maybe it has already been reaped. In both cases an
exception would be nice, or if you decide
On 25/06/2012 09:56, Thomas Stüfe wrote:
Hi, Rob,
src/solaris/native/java/lang/UNIXProcess_md.c:
You never check the return code of kill(2). kill() may fail if you
have no permission to kill the process (EPERM), or if the process id
is invalid, maybe it has already been reaped. In both cases
On 24/06/2012 13:57, Rob McKenna wrote:
Hi folks,
5th, and hopefully final review has been posted to:
http://cr.openjdk.java.net/~robm/4244896/webrev.05/
http://cr.openjdk.java.net/%7Erobm/4244896/webrev.05/
Let me know if there are any comments or concerns, and thanks a lot
for the help
Hi Rob,
A couple of minor things ...
Process.waitFor:
+ long startTime = System.nanoTime();
+ long rem = unit.toNanos(timeout) - (System.nanoTime() -
startTime);
should just be
+ long startTime = System.nanoTime();
+ long rem = unit.toNanos(timeout);
The
On 31/05/2012 17:48, Rob McKenna wrote:
That link should be:
http://cr.openjdk.java.net/~robm/4244896/webrev.04/
-Rob
I'm happy the spec has come good too. One small suggestion is to tweak
this line:
{@code Process} objects returned by {@link ProcessBuilder#start} and
{@link
That link should be:
http://cr.openjdk.java.net/~robm/4244896/webrev.04/
-Rob
On 31/05/12 16:05, Rob McKenna wrote:
Latest version including work on the spec language:
http://cr.openjdk.java.net/~robm/4244896/webrev.04/
http://cr.openjdk.java.net/%7Erobm/4244896/webrev.03/
-Rob
Hi.
185 public boolean waitFor(long timeout, TimeUnit unit)
186 throws InterruptedException {
187 long now = System.nanoTime();
188
189 long end = now +
190 (timeout = 0 ? 0 : TimeUnit.NANOSECONDS.convert(timeout,
unit));
191
Hi Rob,
This looks good to me. I'm glad to see that destroyForcibly mandates
that Process instances from ProcessBuilder.start and Runtime.exec must
do a forcible destroy. That addresses my concern about documenting the
actual implementations.
Two minor comments:
Process.java:
236 *
Jeff,
On 1/06/2012 10:19 AM, Jeff Hain wrote:
Hi.
185 public boolean waitFor(long timeout, TimeUnit unit)
186 throws InterruptedException {
187 long now = System.nanoTime();
188
189 long end = now +
190 (timeout= 0 ? 0 :
Hi Rob,
Your specification for Process.waitFor(long timeout, TimeUnit unit) has
a conflict. You presently say:
If the subprocess has already exited then this method returns
immediately with the value {@code true}
and you also say:
If the current thread:
has its interrupted status set on
Hi Rob,
I dunno if the following has been pointed out before. It's hard to track review
comments.
The implementation of Process.waitFor normalizes the end time and duration
remaining time to nano seconds. However, the Thread.sleep method expects a
duration in units of milliseconds.
You
Hi Paul,
Comments inline:
On 11/05/12 12:29, Paul Sandoz wrote:
Hi Rob,
I dunno if the following has been pointed out before. It's hard to track review
comments.
The implementation of Process.waitFor normalizes the end time and duration
remaining time to nano seconds. However, the
On May 11, 2012, at 5:24 PM, Rob McKenna wrote:
For the following code:
227 long rem = end - now;
228 while (!hasExited (rem 0)) {
229 wait(TimeUnit.NANOSECONDS.toMillis(rem));
230 rem = end - System.nanoTime();
231 }
Can the
Hi folks,
The latest version is at:
http://cr.openjdk.java.net/~robm/4244896/webrev.03/
http://cr.openjdk.java.net/%7Erobm/4244896/webrev.03/
Feedback greatly appreciated.
-Rob
On 19/04/12 12:05, Alan Bateman wrote:
On 19/04/2012 01:05, David Holmes wrote:
On 18/04/2012 11:44 PM,
The process reaper thread will be calling process.notifyAll() so this is
not simply a sleep. In which case the correct form would be:
public synchronized boolean waitFor(long timeout, TimeUnit unit) {
...
while (!hasExited) {
wait(timeLeft);
if (!hasExited) {
timeleft
Rob,
You can't use wait this way:
217 public synchronized boolean waitFor(long timeout, TimeUnit unit)
218 throws InterruptedException {
219 long millis = unit.toMillis(timeout);
220 long nanos = unit.toNanos(timeout) % (millis * 100);
221
222 if
Correction:
On 20/04/2012 7:15 PM, David Holmes wrote:
Rob,
You can't use wait this way:
217 public synchronized boolean waitFor(long timeout, TimeUnit unit)
218 throws InterruptedException {
219 long millis = unit.toMillis(timeout);
220 long nanos = unit.toNanos(timeout) % (millis *
Rob,
2) As Alan noted, there is really no need for isAlive() if people are
happy with the idea of waitFor(long, TimeUnit). I'd appreciate any
feedback on this aspect of the fix.
Process.isAlive is similar to Future.isDone(). I think isAlive fills that need
to have a simple query method
Please avoid the cross package dependency on j.u.TimeUnit.
Keeping mind that in Java ME we have to subset the API to make it fit
and that forciblyDestroy is needed iit would be cleaner to avoid the
dependency
unless there will be a form of the method that uses only a fixed time
unit
Thanks a lot for the feedback folks.
Jason, as Alan notes, that makes a lot of sense. Thanks for that.
I'm planning to do the following over the next couple of days:
- fix UnixProcess.waitFor(long, TimeUnit) as per David's mail.
- alter the default waitFor(long, TimeUnit)
On 19/04/2012 01:05, David Holmes wrote:
On 18/04/2012 11:44 PM, Jason Mehrens wrote:
Rob,
It looks like waitFor is calling Object.wait(long) without owning
this objects monitor. If I pass Long.MAX_VALUE to waitFor, shouldn't
waitFor return if the early if the process ends?
Also waitFor
I've uploaded another webrev to:
http://cr.openjdk.java.net/~robm/4244896/webrev.02/
http://cr.openjdk.java.net/%7Erobm/4244896/webrev.02/
I plan to spend some time over the coming day or two beefing up the test
for waitFor (right now its really geared towards destroyForcibly) so I
won't
Hi Rob,
On 20/04/2012 11:33 AM, Rob McKenna wrote:
I've uploaded another webrev to:
http://cr.openjdk.java.net/~robm/4244896/webrev.02/
http://cr.openjdk.java.net/%7Erobm/4244896/webrev.02/
I'll take a look as soon as I have a chance but will be OOTO the rest of
today.
I plan to spend
Thanks David,
I suspect Alan simply meant that it may be preferable to use
System.nanoTime to measure the time elapsed in the default
implementation as opposed to System.currentTimeMillis. I had forgotten
about the lower resolution of the timer interrupt. I'll revert that
aspect of the
...@oracle.com
Subject: Re: review request: 4244896: (process) Provide System.getPid(),
System.killProcess(String pid)
CC: core-libs-dev@openjdk.java.net
New webrev at:
http://cr.openjdk.java.net/~robm/4244896/webrev.01/
Differences:
- implemented Process.waitFor(long timeout). I figured I'd
if the process ends?
Jason
Date: Tue, 17 Apr 2012 15:56:30 +0100
From: rob.mcke...@oracle.com
To: alan.bate...@oracle.com
Subject: Re: review request: 4244896: (process) Provide
System.getPid(), System.killProcess(String pid)
CC: core-libs-dev@openjdk.java.net
New webrev at:
http
Jason
Date: Tue, 17 Apr 2012 15:56:30 +0100
From: rob.mcke...@oracle.com
To: alan.bate...@oracle.com
Subject: Re: review request: 4244896: (process) Provide System.getPid(),
System.killProcess(String pid)
CC: core-libs-dev@openjdk.java.net
New webrev at:
http://cr.openjdk.java.net/~robm
New webrev at:
http://cr.openjdk.java.net/~robm/4244896/webrev.01/
Differences:
- implemented Process.waitFor(long timeout). I figured I'd throw it in
and let you decide if you want to keep it / replace isAlive.
- implemented defaults in Process.java
- destroyForcibly now returns the Process
On 12/04/2012 00:48, Rob McKenna wrote:
Hi folks,
I'm hoping for some feedback on the above. Webrev:
http://cr.openjdk.java.net/~robm/4244896/webrev.00/
Thanks for taking this one on, a destroyForcibly is really useful to
have (destroy was always misleading given that that is was actually a
30 matches
Mail list logo
