From 2fa31573c4860069c4e17bd2be89eff3a9234737 Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 13 Dec 2019 08:02:48 +0000 Subject: [PATCH] 8208715: Conversion of milliseconds to nanoseconds in UNIXProcess contains bug Reviewed-by: martin, andrew --- .../classes/java/lang/UNIXProcess.java | 3 +- .../classes/java/lang/ProcessImpl.java | 8 +++-- src/windows/native/java/lang/ProcessImpl_md.c | 4 +-- test/java/lang/ProcessBuilder/Basic.java | 34 ++++++++++++++++++- 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/solaris/classes/java/lang/UNIXProcess.java b/src/solaris/classes/java/lang/UNIXProcess.java index 1793a8f35..fe26fd893 100644 --- a/src/solaris/classes/java/lang/UNIXProcess.java +++ b/src/solaris/classes/java/lang/UNIXProcess.java @@ -408,8 +408,7 @@ final class UNIXProcess extends Process { long deadline = System.nanoTime() + remainingNanos; do { - // Round up to next millisecond - wait(TimeUnit.NANOSECONDS.toMillis(remainingNanos + 999_999L)); + TimeUnit.NANOSECONDS.timedWait(this, remainingNanos); if (hasExited) { return true; } diff --git a/src/windows/classes/java/lang/ProcessImpl.java b/src/windows/classes/java/lang/ProcessImpl.java index 37c323754..1970260e5 100644 --- a/src/windows/classes/java/lang/ProcessImpl.java +++ b/src/windows/classes/java/lang/ProcessImpl.java @@ -520,11 +520,15 @@ final class ProcessImpl extends Process { if (timeout <= 0) return false; long remainingNanos = unit.toNanos(timeout); - long deadline = System.nanoTime() + remainingNanos ; + long deadline = System.nanoTime() + remainingNanos; do { // Round up to next millisecond long msTimeout = TimeUnit.NANOSECONDS.toMillis(remainingNanos + 999_999L); + if (msTimeout < 0) { + // if wraps around then wait a long while + msTimeout = Integer.MAX_VALUE; + } waitForTimeoutInterruptibly(handle, msTimeout); if (Thread.interrupted()) throw new InterruptedException(); @@ -538,7 +542,7 @@ final class ProcessImpl extends Process { } private static native void waitForTimeoutInterruptibly( - long handle, long timeout); + long handle, long timeoutMillis); public void destroy() { terminateProcess(handle); } diff --git a/src/windows/native/java/lang/ProcessImpl_md.c b/src/windows/native/java/lang/ProcessImpl_md.c index 3a013c159..8dcf8972e 100644 --- a/src/windows/native/java/lang/ProcessImpl_md.c +++ b/src/windows/native/java/lang/ProcessImpl_md.c @@ -421,10 +421,10 @@ JNIEXPORT void JNICALL Java_java_lang_ProcessImpl_waitForTimeoutInterruptibly(JNIEnv *env, jclass ignored, jlong handle, - jlong timeout) + jlong timeoutMillis) { HANDLE events[2]; - DWORD dwTimeout = (DWORD)timeout; + DWORD dwTimeout = (DWORD)timeoutMillis; DWORD result; events[0] = (HANDLE) handle; events[1] = JVM_GetThreadInterruptEvent(); diff --git a/test/java/lang/ProcessBuilder/Basic.java b/test/java/lang/ProcessBuilder/Basic.java index 7ea8a52b8..7a9260acb 100644 --- a/test/java/lang/ProcessBuilder/Basic.java +++ b/test/java/lang/ProcessBuilder/Basic.java @@ -2316,6 +2316,7 @@ public class Basic { public void run() { try { aboutToWaitFor.countDown(); + Thread.currentThread().interrupt(); boolean result = p.waitFor(30L * 1000L, TimeUnit.MILLISECONDS); fail("waitFor() wasn't interrupted, its return value was: " + result); } catch (InterruptedException success) { @@ -2325,7 +2326,38 @@ public class Basic { thread.start(); aboutToWaitFor.await(); - Thread.sleep(1000); + thread.interrupt(); + thread.join(10L * 1000L); + check(millisElapsedSince(start) < 10L * 1000L); + check(!thread.isAlive()); + p.destroy(); + } catch (Throwable t) { unexpected(t); } + + //---------------------------------------------------------------- + // Check that Process.waitFor(Long.MAX_VALUE, TimeUnit.MILLISECONDS) + // interrupt works as expected, if interrupted while waiting. + //---------------------------------------------------------------- + try { + List childArgs = new ArrayList(javaChildArgs); + childArgs.add("sleep"); + final Process p = new ProcessBuilder(childArgs).start(); + final long start = System.nanoTime(); + final CountDownLatch aboutToWaitFor = new CountDownLatch(1); + + final Thread thread = new Thread() { + public void run() { + try { + aboutToWaitFor.countDown(); + Thread.currentThread().interrupt(); + boolean result = p.waitFor(Long.MAX_VALUE, TimeUnit.MILLISECONDS); + fail("waitFor() wasn't interrupted, its return value was: " + result); + } catch (InterruptedException success) { + } catch (Throwable t) { unexpected(t); } + } + }; + + thread.start(); + aboutToWaitFor.await(); thread.interrupt(); thread.join(10L * 1000L); check(millisElapsedSince(start) < 10L * 1000L); -- GitLab