diff options
Diffstat (limited to 'src/main/java/com/google/devtools')
4 files changed, 112 insertions, 70 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/shell/Command.java b/src/main/java/com/google/devtools/build/lib/shell/Command.java index 84f78d2ae3..79b850acad 100644 --- a/src/main/java/com/google/devtools/build/lib/shell/Command.java +++ b/src/main/java/com/google/devtools/build/lib/shell/Command.java @@ -819,9 +819,6 @@ public final class Command { TerminationStatus status = waitForProcess(process, killSubprocessOnInterrupt); observer.stopObserving(processKillable); - // #close() must be called after the #stopObserving() so that a badly-timed timeout does not - // try to destroy a process that is already closed - process.close(); log.finer(status.toString()); @@ -849,6 +846,11 @@ public final class Command { : new AbnormalTerminationException(this, noOutputResult, message, ioe); } + } finally { + // #close() must be called after the #stopObserving() so that a badly-timed timeout does not + // try to destroy a process that is already closed, and after outErr is completed, + // so that it has a chance to read the entire output is captured. + process.close(); } CommandResult result = diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsProcesses.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsProcesses.java index d8e07a0a5c..2188068245 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/WindowsProcesses.java +++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsProcesses.java @@ -20,6 +20,8 @@ import java.util.List; * Process management on Windows. */ public class WindowsProcesses { + public static final long INVALID = -1; + private static boolean jniLoaded = false; private WindowsProcesses() { // Prevent construction @@ -57,23 +59,21 @@ public class WindowsProcesses { */ static native int nativeWriteStdin(long process, byte[] bytes, int offset, int length); - /** - * Reads data from the stdout of the specified process into the given array. - * - * <p>Blocks until either some data was read or the process is terminated. - * - * @return the number of bytes read or -1 if there was an error. - */ - static native int nativeReadStdout(long process, byte[] bytes, int offset, int length); + /** Returns an opaque identifier of stdout stream for the process. */ + static native long nativeGetStdout(long process); + + /** Returns am opaque identifier of stderr stream for the process. */ + static native long nativeGetStderr(long process); /** - * Reads data from the stderr of the specified process into the given array. + * Reads data from the stream into the given array. {@code stream} should come from {@link + * #nativeGetStdout(long)} or {@link #nativeGetStderr(long)}. * * <p>Blocks until either some data was read or the process is terminated. * - * @return the number of bytes read or -1 if there was an error. + * @return the number of bytes read, 0 on EOF, or -1 if there was an error. */ - static native int nativeReadStderr(long process, byte[] bytes, int offset, int length); + static native int nativeReadStream(long stream, byte[] bytes, int offset, int length); /** * Waits until the given process terminates. @@ -101,21 +101,31 @@ public class WindowsProcesses { /** * Releases the native data structures associated with the process. * - * <p>Calling any other method on the same process after this call will result in the JVM - * crashing or worse. + * <p>Calling any other method on the same process after this call will result in the JVM crashing + * or worse. + */ + static native void nativeDeleteProcess(long process); + + /** + * Closes the stream + * + * @param stream should come from {@link #nativeGetStdout(long)} or {@link + * #nativeGetStderr(long)}. */ - static native void nativeDelete(long process); + static native void nativeCloseStream(long stream); /** - * Returns a string representation of the last error caused by any call on the given process - * or the empty string if the last operation was successful. + * Returns a string representation of the last error caused by any call on the given process or + * the empty string if the last operation was successful. * * <p>Does <b>NOT</b> terminate the process if it is still running. * * <p>After this call returns, subsequent calls will return the empty string if there was no * failed operation in between. */ - static native String nativeGetLastError(long process); + static native String nativeProcessGetLastError(long process); + + static native String nativeStreamGetLastError(long process); public static int getpid() { ensureJni(); diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java index df7b11c8fc..245774911e 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java +++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java @@ -29,7 +29,8 @@ import java.util.concurrent.atomic.AtomicInteger; * A Windows subprocess backed by a native object. */ public class WindowsSubprocess implements Subprocess { - private enum Stream { OUT, ERR }; + // For debugging purposes. + private String commandLine; /** * Output stream for writing to the stdin of a Windows process. @@ -52,12 +53,14 @@ public class WindowsSubprocess implements Subprocess { /** * Input stream for reading the stdout or stderr of a Windows process. + * + * <p>This class is non-static for debugging purposes. */ private class ProcessInputStream extends InputStream { - private final Stream stream; + private long nativeStream; - ProcessInputStream(Stream stream) { - this.stream = stream; + ProcessInputStream(long nativeStream) { + this.nativeStream = nativeStream; } @Override @@ -71,8 +74,35 @@ public class WindowsSubprocess implements Subprocess { } @Override - public int read(byte b[], int off, int len) throws IOException { - return readStream(stream, b, off, len); + public synchronized int read(byte b[], int off, int len) throws IOException { + if (nativeStream == WindowsProcesses.INVALID) { + throw new IllegalStateException(); + } + + int result = WindowsProcesses.nativeReadStream(nativeStream, b, off, len); + + if (result == 0) { + return -1; // EOF + } + if (result == -1) { + throw new IOException(WindowsProcesses.nativeStreamGetLastError(nativeStream)); + } + + return result; + } + + @Override + public synchronized void close() { + if (nativeStream != WindowsProcesses.INVALID) { + WindowsProcesses.nativeCloseStream(nativeStream); + nativeStream = WindowsProcesses.INVALID; + } + } + + @Override + protected void finalize() throws Throwable { + close(); + super.finalize(); } } @@ -89,17 +119,26 @@ public class WindowsSubprocess implements Subprocess { } }); - private long nativeProcess; - private final OutputStream outputStream; - private final InputStream inputStream; - private final InputStream errorStream; + private volatile long nativeProcess; + private final OutputStream stdinStream; + private final ProcessInputStream stdoutStream; + private final ProcessInputStream stderrStream; private final CountDownLatch waitLatch; - WindowsSubprocess(long nativeProcess, boolean stdoutRedirected, boolean stderrRedirected) { + + WindowsSubprocess( + long nativeProcess, String commandLine, boolean stdoutRedirected, boolean stderrRedirected) { + this.commandLine = commandLine; this.nativeProcess = nativeProcess; - inputStream = stdoutRedirected ? null : new ProcessInputStream(Stream.OUT); - errorStream = stderrRedirected ? null : new ProcessInputStream(Stream.ERR); - outputStream = new ProcessOutputStream(); + stdoutStream = + stdoutRedirected + ? null + : new ProcessInputStream(WindowsProcesses.nativeGetStdout(nativeProcess)); + stderrStream = + stderrRedirected + ? null + : new ProcessInputStream(WindowsProcesses.nativeGetStderr(nativeProcess)); + stdinStream = new ProcessOutputStream(); waitLatch = new CountDownLatch(1); // Every Windows process we start consumes a thread here. This is suboptimal, but seems to be // the sanest way to reconcile WaitForMultipleObjects() and Java-style interruption. @@ -122,8 +161,11 @@ public class WindowsSubprocess implements Subprocess { } @Override - public synchronized void finalize() { - close(); + public synchronized void finalize() throws Throwable { + if (nativeProcess != WindowsProcesses.INVALID) { + close(); + } + super.finalize(); } @Override @@ -142,7 +184,7 @@ public class WindowsSubprocess implements Subprocess { checkLiveness(); int result = WindowsProcesses.nativeGetExitCode(nativeProcess); - String error = WindowsProcesses.nativeGetLastError(nativeProcess); + String error = WindowsProcesses.nativeProcessGetLastError(nativeProcess); if (!error.isEmpty()) { throw new IllegalStateException(error); } @@ -162,46 +204,28 @@ public class WindowsSubprocess implements Subprocess { @Override public synchronized void close() { - if (nativeProcess != -1) { - WindowsProcesses.nativeDelete(nativeProcess); - nativeProcess = -1; + if (nativeProcess != WindowsProcesses.INVALID) { + stdoutStream.close(); + stderrStream.close(); + long process = nativeProcess; + nativeProcess = WindowsProcesses.INVALID; + WindowsProcesses.nativeDeleteProcess(process); } } @Override public OutputStream getOutputStream() { - return outputStream; + return stdinStream; } @Override public InputStream getInputStream() { - return inputStream; + return stdoutStream; } @Override public InputStream getErrorStream() { - return errorStream; - } - - private synchronized int readStream(Stream stream, byte b[], int off, int len) - throws IOException { - checkLiveness(); - - int result = -1; - switch (stream) { - case OUT: - result = WindowsProcesses.nativeReadStdout(nativeProcess, b, off, len); - break; - case ERR: - result = WindowsProcesses.nativeReadStderr(nativeProcess, b, off, len); - break; - } - - if (result == -1) { - throw new IOException(WindowsProcesses.nativeGetLastError(nativeProcess)); - } - - return result; + return stderrStream; } private synchronized void writeStream(byte[] b, int off, int len) throws IOException { @@ -215,7 +239,7 @@ public class WindowsSubprocess implements Subprocess { // I think the Windows API never returns 0 in dwNumberOfBytesWritten // Verify.verify(written != 0); if (written == -1) { - throw new IOException(WindowsProcesses.nativeGetLastError(nativeProcess)); + throw new IOException(WindowsProcesses.nativeProcessGetLastError(nativeProcess)); } remaining -= written; @@ -224,8 +248,13 @@ public class WindowsSubprocess implements Subprocess { } private void checkLiveness() { - if (nativeProcess == -1) { + if (nativeProcess == WindowsProcesses.INVALID) { throw new IllegalStateException(); } } + + @Override + public String toString() { + return String.format("%s:[%s]", super.toString(), commandLine); + } } diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java index 7b4a7d8b7e..10759a91ce 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java +++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java @@ -37,7 +37,7 @@ public class WindowsSubprocessFactory implements Subprocess.Factory { @Override public Subprocess create(SubprocessBuilder builder) throws IOException { WindowsJniLoader.loadJni(); - + String commandLine = WindowsProcesses.quoteCommandLine(builder.getArgv()); byte[] env = builder.getEnv() == null ? null : convertEnvToNative(builder.getEnv()); @@ -46,13 +46,14 @@ public class WindowsSubprocessFactory implements Subprocess.Factory { long nativeProcess = WindowsProcesses.nativeCreateProcess( commandLine, env, builder.getWorkingDirectory().getPath(), stdoutPath, stderrPath); - String error = WindowsProcesses.nativeGetLastError(nativeProcess); + String error = WindowsProcesses.nativeProcessGetLastError(nativeProcess); if (!error.isEmpty()) { - WindowsProcesses.nativeDelete(nativeProcess); + WindowsProcesses.nativeDeleteProcess(nativeProcess); throw new IOException(error); } - return new WindowsSubprocess(nativeProcess, stdoutPath != null, stderrPath != null); + return new WindowsSubprocess( + nativeProcess, commandLine, stdoutPath != null, stderrPath != null); } private String getRedirectPath(StreamAction action, File file) { |