8333742: ProcessImpl and ProcessHandleImpl may mishandle processes that exit with code 259

Reviewed-by: rriggs
This commit is contained in:
Daniel Jeliński 2024-06-11 17:34:09 +00:00
parent aaaa86b571
commit b77bd5fd6a
4 changed files with 34 additions and 31 deletions
src/java.base/windows
test/jdk/java/lang/ProcessHandle

@ -557,8 +557,14 @@ final class ProcessImpl extends Process {
public int exitValue() {
int exitCode = getExitCodeProcess(handle);
if (exitCode == STILL_ACTIVE)
throw new IllegalThreadStateException("process has not exited");
if (exitCode == STILL_ACTIVE) {
// STILL_ACTIVE (259) might be the real exit code
if (isProcessAlive(handle)) {
throw new IllegalThreadStateException("process has not exited");
}
// call again, in case the process just exited
return getExitCodeProcess(handle);
}
return exitCode;
}
private static native int getExitCodeProcess(long handle);
@ -582,7 +588,7 @@ final class ProcessImpl extends Process {
throws InterruptedException
{
long remainingNanos = unit.toNanos(timeout); // throw NPE before other conditions
if (getExitCodeProcess(handle) != STILL_ACTIVE) return true;
if (!isProcessAlive(handle)) return true;
if (timeout <= 0) return false;
long deadline = System.nanoTime() + remainingNanos;
@ -601,13 +607,13 @@ final class ProcessImpl extends Process {
}
if (Thread.interrupted())
throw new InterruptedException();
if (getExitCodeProcess(handle) != STILL_ACTIVE) {
if (!isProcessAlive(handle)) {
return true;
}
remainingNanos = deadline - System.nanoTime();
} while (remainingNanos > 0);
return (getExitCodeProcess(handle) != STILL_ACTIVE);
return !isProcessAlive(handle);
}
private static native void waitForTimeoutInterruptibly(

@ -1,5 +1,5 @@
/*
* Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -109,27 +109,19 @@ Java_java_lang_ProcessHandleImpl_waitForProcessExit0(JNIEnv* env,
if (handle == NULL) {
return exitValue; // No process with that pid is alive
}
do {
if (!GetExitCodeProcess(handle, &exitValue)) {
if (!GetExitCodeProcess(handle, &exitValue)) {
JNU_ThrowByNameWithLastError(env,
"java/lang/RuntimeException", "GetExitCodeProcess");
} else if (exitValue == STILL_ACTIVE) {
if (WaitForSingleObject(handle, INFINITE) /* Wait forever */
== WAIT_FAILED) {
JNU_ThrowByNameWithLastError(env,
"java/lang/RuntimeException", "WaitForSingleObjects");
} else if (!GetExitCodeProcess(handle, &exitValue)) {
JNU_ThrowByNameWithLastError(env,
"java/lang/RuntimeException", "GetExitCodeProcess");
break;
}
if (exitValue == STILL_ACTIVE) {
HANDLE events[2];
events[0] = handle;
events[1] = JVM_GetThreadInterruptEvent();
if (WaitForMultipleObjects(sizeof(events)/sizeof(events[0]), events,
FALSE, /* Wait for ANY event */
INFINITE) /* Wait forever */
== WAIT_FAILED) {
JNU_ThrowByNameWithLastError(env,
"java/lang/RuntimeException", "WaitForMultipleObjects");
break;
}
}
} while (exitValue == STILL_ACTIVE);
}
CloseHandle(handle); // Ignore return code
return exitValue;
}

@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -467,9 +467,8 @@ Java_java_lang_ProcessImpl_terminateProcess(JNIEnv *env, jclass ignored, jlong h
JNIEXPORT jboolean JNICALL
Java_java_lang_ProcessImpl_isProcessAlive(JNIEnv *env, jclass ignored, jlong handle)
{
DWORD dwExitStatus;
GetExitCodeProcess((HANDLE) handle, &dwExitStatus);
return dwExitStatus == STILL_ACTIVE;
return WaitForSingleObject((HANDLE) handle, 0) /* don't wait */
== WAIT_TIMEOUT;
}
JNIEXPORT jboolean JNICALL

@ -1,5 +1,5 @@
/*
* Copyright (c) 2014, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -32,6 +32,7 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import jdk.test.lib.Platform;
import jdk.test.lib.Utils;
import org.testng.annotations.Test;
@ -40,6 +41,7 @@ import org.testng.TestNG;
/*
* @test
* @bug 8333742
* @requires vm.flagless
* @library /test/lib
* @modules jdk.management
@ -65,8 +67,10 @@ public class OnExitTest extends ProcessUtil {
@Test
public static void test1() {
try {
int[] exitValues = {0, 1, 10};
int[] exitValues = {0, 1, 10, 259};
for (int value : exitValues) {
// Linux & Mac don't support exit codes longer than 8 bits, skip
if (value == 259 && !Platform.isWindows()) continue;
Process p = JavaChild.spawn("exit", Integer.toString(value));
CompletableFuture<Process> future = p.onExit();
future.thenAccept( (ph) -> {
@ -81,7 +85,9 @@ public class OnExitTest extends ProcessUtil {
Assert.assertEquals(h, p);
Assert.assertEquals(p.exitValue(), value);
Assert.assertFalse(p.isAlive(), "Process should not be alive");
p.waitFor();
Assert.assertEquals(p.waitFor(), value);
Assert.assertTrue(p.waitFor(1, TimeUnit.MILLISECONDS),
"waitFor should return true");
}
} catch (IOException | InterruptedException | ExecutionException ex) {
Assert.fail(ex.getMessage(), ex);