fix Runtime.exec bugs

The first bug affected POSIX systems: if the app never called
Process.waitFor, we'd never call waitpid on the child and thus leak a
zombie process.  This patch ensures that we always call waitpid by
spawning a thread to handle it.

The second bug affected Windows systems: we weren't closing the
child's ends of the stdin, stdout, and stderr pipes after process
creation, which lead to us blocking forever while reading from the
child's stdout or stderr.
This commit is contained in:
Joel Dice
2010-11-09 10:22:23 -07:00
parent 5ade8d1cf6
commit 3d18f88ad9
2 changed files with 70 additions and 48 deletions

View File

@ -191,6 +191,10 @@ Java_java_lang_Runtime_exec(JNIEnv* e, jclass,
BOOL success = CreateProcess(0, (LPSTR) RUNTIME_ARRAY_BODY(line), 0, 0, 1, BOOL success = CreateProcess(0, (LPSTR) RUNTIME_ARRAY_BODY(line), 0, 0, 1,
CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT, CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT,
0, 0, &si, &pi); 0, 0, &si, &pi);
CloseHandle(in[1]);
CloseHandle(out[0]);
CloseHandle(err[1]);
if (not success) { if (not success) {
throwNew(e, "java/io/IOException", getErrorStr(GetLastError())); throwNew(e, "java/io/IOException", getErrorStr(GetLastError()));
@ -202,19 +206,6 @@ Java_java_lang_Runtime_exec(JNIEnv* e, jclass,
} }
extern "C" JNIEXPORT jint JNICALL
Java_java_lang_Runtime_exitValue(JNIEnv* e, jclass, jlong pid)
{
DWORD exitCode;
BOOL success = GetExitCodeProcess(reinterpret_cast<HANDLE>(pid), &exitCode);
if(not success){
throwNew(e, "java/lang/Exception", getErrorStr(GetLastError()));
} else if(exitCode == STILL_ACTIVE){
throwNew(e, "java/lang/IllegalThreadStateException", "Process is still active");
}
return exitCode;
}
extern "C" JNIEXPORT jint JNICALL extern "C" JNIEXPORT jint JNICALL
Java_java_lang_Runtime_waitFor(JNIEnv* e, jclass, jlong pid) Java_java_lang_Runtime_waitFor(JNIEnv* e, jclass, jlong pid)
{ {
@ -317,20 +308,6 @@ Java_java_lang_Runtime_exec(JNIEnv* e, jclass,
fcntl(err[0], F_SETFD, FD_CLOEXEC); fcntl(err[0], F_SETFD, FD_CLOEXEC);
} }
extern "C" JNIEXPORT jint JNICALL
Java_java_lang_Runtime_exitValue(JNIEnv* e, jclass, jlong pid)
{
int status;
pid_t returned = waitpid(pid, &status, WNOHANG);
if(returned == 0){
throwNewErrno(e, "java/lang/IllegalThreadStateException");
} else if(returned == -1){
throwNewErrno(e, "java/lang/Exception");
}
return WEXITSTATUS(status);
}
extern "C" JNIEXPORT jint JNICALL extern "C" JNIEXPORT jint JNICALL
Java_java_lang_Runtime_waitFor(JNIEnv*, jclass, jlong pid) Java_java_lang_Runtime_waitFor(JNIEnv*, jclass, jlong pid)
{ {

View File

@ -43,27 +43,71 @@ public class Runtime {
} }
} }
public Process exec(String command) throws IOException { public Process exec(String command) {
long[] process = new long[4];
StringTokenizer t = new StringTokenizer(command); StringTokenizer t = new StringTokenizer(command);
String[] cmd = new String[t.countTokens()]; String[] cmd = new String[t.countTokens()];
for (int i = 0; i < cmd.length; i++) for (int i = 0; i < cmd.length; i++)
cmd[i] = t.nextToken(); cmd[i] = t.nextToken();
exec(cmd, process);
return new MyProcess(process[0], (int) process[1], (int) process[2], (int) process[3]); return exec(cmd);
} }
public Process exec(String[] command) { public MyProcess exec(final String[] command) {
long[] process = new long[4]; final MyProcess[] process = new MyProcess[1];
exec(command, process); final Throwable[] exception = new Throwable[1];
return new MyProcess(process[0], (int) process[1], (int) process[2], (int) process[3]);
synchronized (process) {
Thread t = new Thread() {
public void run() {
synchronized (process) {
try {
long[] info = new long[4];
exec(command, info);
process[0] = new MyProcess
(info[0], (int) info[1], (int) info[2], (int) info[3]);
MyProcess p = process[0];
synchronized (p) {
try {
if (p.pid != 0) {
p.exitCode = Runtime.waitFor(p.pid);
p.pid = 0;
}
} finally {
p.notifyAll();
}
}
} catch (Throwable e) {
exception[0] = e;
} finally {
process.notifyAll();
}
}
}
};
t.setDaemon(true);
t.start();
while (process[0] == null && exception[0] == null) {
try {
process.wait();
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
}
}
if (exception[0] != null) {
throw new RuntimeException(exception[0]);
}
return process[0];
} }
public native void addShutdownHook(Thread t); public native void addShutdownHook(Thread t);
private static native void exec(String[] command, long[] process); private static native void exec(String[] command, long[] process)
throws IOException;
private static native int exitValue(long pid);
private static native int waitFor(long pid); private static native int waitFor(long pid);
@ -95,13 +139,6 @@ public class Runtime {
throw new RuntimeException("not implemented"); throw new RuntimeException("not implemented");
} }
public int exitValue() {
if (pid != 0) {
exitCode = Runtime.exitValue(pid);
}
return exitCode;
}
public InputStream getInputStream() { public InputStream getInputStream() {
return new FileInputStream(new FileDescriptor(in)); return new FileInputStream(new FileDescriptor(in));
} }
@ -114,11 +151,19 @@ public class Runtime {
return new FileInputStream(new FileDescriptor(err)); return new FileInputStream(new FileDescriptor(err));
} }
public int waitFor() throws InterruptedException { public synchronized int exitValue() {
if (pid != 0) { if (pid != 0) {
exitCode = Runtime.waitFor(pid); throw new IllegalThreadStateException();
pid = 0;
} }
return exitCode;
}
public synchronized int waitFor() throws InterruptedException {
while (pid != 0) {
wait();
}
return exitCode; return exitCode;
} }
} }