From e2f9adab37975cc6565f218b26021afa0796e2fd Mon Sep 17 00:00:00 2001 From: Dmitry Lomov Date: Fri, 8 Jul 2016 14:29:53 +0000 Subject: Fix capturing stdin/stdout on Windows. 1. Return EOF for streams representing Windows process pipes. 2. Fix the timing of process.close() 3. Un-synchronized reading of stderr and stdout. -- Change-Id: Iec98f45db9984be2c2b066962801cbd3ca60da3f Reviewed-on: https://bazel-review.googlesource.com/#/c/4000/ MOS_MIGRATED_REVID=126910063 --- .../google/devtools/build/lib/shell/Command.java | 8 +- .../build/lib/windows/WindowsProcesses.java | 44 ++++--- .../build/lib/windows/WindowsSubprocess.java | 121 ++++++++++------- .../lib/windows/WindowsSubprocessFactory.java | 9 +- src/main/native/windows_processes.cc | 146 ++++++++++++--------- .../build/lib/windows/WindowsProcessesTest.java | 140 +++++++++++--------- 6 files changed, 271 insertions(+), 197 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. - * - *

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)}. * *

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. * - *

Calling any other method on the same process after this call will result in the JVM - * crashing or worse. + *

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. * *

Does NOT terminate the process if it is still running. * *

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. + * + *

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) { diff --git a/src/main/native/windows_processes.cc b/src/main/native/windows_processes.cc index f27ba39259..b5a810eaac 100644 --- a/src/main/native/windows_processes.cc +++ b/src/main/native/windows_processes.cc @@ -55,27 +55,37 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeGetpid( return GetCurrentProcessId(); } +struct NativeOutputStream { + HANDLE handle_; + std::string error_; + + NativeOutputStream() : handle_(INVALID_HANDLE_VALUE), error_("") {} + + void close() { + if (handle_ != INVALID_HANDLE_VALUE) { + CloseHandle(handle_); + handle_ = INVALID_HANDLE_VALUE; + } + } +}; + struct NativeProcess { HANDLE stdin_; - HANDLE stdout_; - HANDLE stderr_; + NativeOutputStream stdout_; + NativeOutputStream stderr_; HANDLE process_; HANDLE job_; - HANDLE event_; std::string error_; - NativeProcess(); + NativeProcess() + : stdin_(INVALID_HANDLE_VALUE), + stdout_(), + stderr_(), + process_(INVALID_HANDLE_VALUE), + job_(INVALID_HANDLE_VALUE), + error_("") {} }; -NativeProcess::NativeProcess() { - stdin_ = INVALID_HANDLE_VALUE; - stdout_ = INVALID_HANDLE_VALUE; - stderr_ = INVALID_HANDLE_VALUE; - process_ = INVALID_HANDLE_VALUE; - job_ = INVALID_HANDLE_VALUE; - error_ = ""; -} - extern "C" JNIEXPORT jlong JNICALL Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( JNIEnv *env, jclass clazz, jstring java_commandline, jbyteArray java_env, @@ -132,15 +142,6 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( } } - event = CreateEvent(NULL, TRUE, FALSE, NULL); - if (event == NULL) { - event = INVALID_HANDLE_VALUE; - result->error_ = GetLastErrorString("CreateEvent()"); - goto cleanup; - } - - result->event_ = event; - if (!CreatePipe(&stdin_process, &result->stdin_, &sa, 0)) { result->error_ = GetLastErrorString("CreatePipe(stdin)"); goto cleanup; @@ -161,7 +162,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( goto cleanup; } } else { - if (!CreatePipe(&result->stdout_, &stdout_process, &sa, 0)) { + if (!CreatePipe(&result->stdout_.handle_, &stdout_process, &sa, 0)) { result->error_ = GetLastErrorString("CreatePipe(stdout)"); goto cleanup; } @@ -186,7 +187,7 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCreateProcess( } } } else { - if (!CreatePipe(&result->stderr_, &stderr_process, &sa, 0)) { + if (!CreatePipe(&result->stderr_.handle_, &stderr_process, &sa, 0)) { result->error_ = GetLastErrorString("CreatePipe(stderr)"); goto cleanup; } @@ -322,49 +323,57 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeWriteStdin( return bytes_written; } -jint ReadFromHandle(HANDLE handle, NativeProcess* process, JNIEnv* env, - jbyteArray java_bytes, jint offset, jint length) { - if (handle == INVALID_HANDLE_VALUE) { - process->error_ = "File handle closed"; +extern "C" JNIEXPORT jlong JNICALL +Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeGetStdout( + JNIEnv* env, jclass clazz, jlong process_long) { + NativeProcess* process = reinterpret_cast(process_long); + return reinterpret_cast(&process->stdout_); +} + +extern "C" JNIEXPORT jlong JNICALL +Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeGetStderr( + JNIEnv* env, jclass clazz, jlong process_long) { + NativeProcess* process = reinterpret_cast(process_long); + return reinterpret_cast(&process->stderr_); +} + +extern "C" JNIEXPORT jint JNICALL +Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeReadStream( + JNIEnv* env, jclass clazz, jlong stream_long, jbyteArray java_bytes, + jint offset, jint length) { + NativeOutputStream* stream = + reinterpret_cast(stream_long); + + if (stream->handle_ == INVALID_HANDLE_VALUE) { + stream->error_ = "File handle closed"; return -1; } jsize array_size = env->GetArrayLength(java_bytes); if (offset < 0 || length <= 0 || offset > array_size - length) { - process->error_ = "Array index out of bounds"; + stream->error_ = "Array index out of bounds"; return -1; } jbyte* bytes = env->GetByteArrayElements(java_bytes, NULL); DWORD bytes_read; - if (!ReadFile(handle, bytes + offset, length, &bytes_read, NULL)) { - process->error_ = GetLastErrorString("ReadFile()"); - bytes_read = -1; + if (!ReadFile(stream->handle_, bytes + offset, length, &bytes_read, NULL)) { + if (GetLastError() == ERROR_BROKEN_PIPE) { + // End of file. + stream->error_ = ""; + bytes_read = 0; + } else { + stream->error_ = GetLastErrorString("ReadFile()"); + bytes_read = -1; + } + } else { + stream->error_ = ""; } env->ReleaseByteArrayElements(java_bytes, bytes, 0); - process->error_ = ""; return bytes_read; } -extern "C" JNIEXPORT jint JNICALL -Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeReadStdout( - JNIEnv *env, jclass clazz, jlong process_long, jbyteArray java_bytes, - jint offset, jint length) { - NativeProcess* process = reinterpret_cast(process_long); - return ReadFromHandle(process->stdout_, process, env, java_bytes, offset, - length); -} - -extern "C" JNIEXPORT jint JNICALL -Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeReadStderr( - JNIEnv *env, jclass clazz, jlong process_long, jbyteArray java_bytes, - jint offset, jint length) { - NativeProcess* process = reinterpret_cast(process_long); - return ReadFromHandle(process->stderr_, process, env, java_bytes, offset, - length); -} - extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeGetExitCode( JNIEnv *env, jclass clazz, jlong process_long) { @@ -421,21 +430,16 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeTerminate( } extern "C" JNIEXPORT void JNICALL -Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeDelete( - JNIEnv *env, jclass clazz, jlong process_long) { +Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeDeleteProcess( + JNIEnv* env, jclass clazz, jlong process_long) { NativeProcess* process = reinterpret_cast(process_long); if (process->stdin_ != INVALID_HANDLE_VALUE) { CloseHandle(process->stdin_); } - if (process->stdout_ != INVALID_HANDLE_VALUE) { - CloseHandle(process->stdout_); - } - - if (process->stderr_ != INVALID_HANDLE_VALUE) { - CloseHandle(process->stderr_); - } + process->stdout_.close(); + process->stderr_.close(); if (process->process_ != INVALID_HANDLE_VALUE) { CloseHandle(process->process_); @@ -448,11 +452,29 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeDelete( delete process; } +extern "C" JNIEXPORT void JNICALL +Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeCloseStream( + JNIEnv* env, jclass clazz, jlong stream_long) { + NativeOutputStream* stream = + reinterpret_cast(stream_long); + stream->close(); +} + extern "C" JNIEXPORT jstring JNICALL -Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeGetLastError( - JNIEnv *env, jclass clazz, jlong process_long) { +Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeProcessGetLastError( + JNIEnv* env, jclass clazz, jlong process_long) { NativeProcess* process = reinterpret_cast(process_long); jstring result = env->NewStringUTF(process->error_.c_str()); process->error_ = ""; return result; } + +extern "C" JNIEXPORT jstring JNICALL +Java_com_google_devtools_build_lib_windows_WindowsProcesses_nativeStreamGetLastError( + JNIEnv* env, jclass clazz, jlong stream_long) { + NativeOutputStream* stream = + reinterpret_cast(stream_long); + jstring result = env->NewStringUTF(stream->error_.c_str()); + stream->error_ = ""; + return result; +} diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java index 30159fc376..07f88d077a 100644 --- a/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java +++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java @@ -61,7 +61,7 @@ public class WindowsProcessesTest { public void terminateProcess() throws Exception { if (process != -1) { WindowsProcesses.nativeTerminate(process); - WindowsProcesses.nativeDelete(process); + WindowsProcesses.nativeDeleteProcess(process); process = -1; } } @@ -78,21 +78,25 @@ public class WindowsProcessesTest { return WindowsProcesses.quoteCommandLine(argv); } - private void assertNoError() throws Exception { - assertThat(WindowsProcesses.nativeGetLastError(process)).isEmpty(); + private void assertNoProcessError() throws Exception { + assertThat(WindowsProcesses.nativeProcessGetLastError(process)).isEmpty(); + } + + private void assertNoStreamError(long stream) throws Exception { + assertThat(WindowsProcesses.nativeStreamGetLastError(stream)).isEmpty(); } @Test public void testSmoke() throws Exception { process = WindowsProcesses.nativeCreateProcess(mockArgs("Ia5", "Oa"), null, null, null, null); - assertNoError(); + assertNoProcessError(); byte[] input = "HELLO".getBytes(UTF8); byte[] output = new byte[5]; WindowsProcesses.nativeWriteStdin(process, input, 0, 5); - assertNoError(); - WindowsProcesses.nativeReadStdout(process, output, 0, 5); - assertNoError(); + assertNoProcessError(); + readStdout(output, 0, 5); + assertNoStreamError(WindowsProcesses.nativeGetStdout(process)); assertThat(new String(output, UTF8)).isEqualTo("HELLO"); } @@ -111,17 +115,27 @@ public class WindowsProcessesTest { assertThat(input.length).isEqualTo(3); assertThat(WindowsProcesses.nativeWriteStdin(process, input, 0, 3)).isEqualTo(3); byte[] output = new byte[3]; - assertThat(WindowsProcesses.nativeReadStdout(process, output, 0, 3)).isEqualTo(3); + assertThat(readStdout(output, 0, 3)).isEqualTo(3); assertThat(Integer.parseInt(new String(output, UTF8))).isEqualTo(i); } } + private int readStdout(byte[] output, int offset, int length) { + return WindowsProcesses.nativeReadStream( + WindowsProcesses.nativeGetStdout(process), output, offset, length); + } + + private int readStderr(byte[] output, int offset, int length) { + return WindowsProcesses.nativeReadStream( + WindowsProcesses.nativeGetStderr(process), output, offset, length); + } + @Test public void testExitCode() throws Exception { process = WindowsProcesses.nativeCreateProcess(mockArgs("X42"), null, null, null, null); assertThat(WindowsProcesses.nativeWaitFor(process)).isTrue(); assertThat(WindowsProcesses.nativeGetExitCode(process)).isEqualTo(42); - assertNoError(); + assertNoProcessError(); } @Test @@ -130,10 +144,10 @@ public class WindowsProcessesTest { byte[] one = new byte[2]; byte[] two = new byte[3]; - assertThat(WindowsProcesses.nativeReadStdout(process, one, 0, 2)).isEqualTo(2); - assertNoError(); - assertThat(WindowsProcesses.nativeReadStdout(process, two, 0, 3)).isEqualTo(3); - assertNoError(); + assertThat(readStdout(one, 0, 2)).isEqualTo(2); + assertNoStreamError(WindowsProcesses.nativeGetStdout(process)); + assertThat(readStdout(two, 0, 3)).isEqualTo(3); + assertNoStreamError(WindowsProcesses.nativeGetStdout(process)); assertThat(new String(one, UTF8)).isEqualTo("HE"); assertThat(new String(two, UTF8)).isEqualTo("LLO"); @@ -143,22 +157,18 @@ public class WindowsProcessesTest { public void testArrayOutOfBounds() throws Exception { process = WindowsProcesses.nativeCreateProcess(mockArgs("O-oob"), null, null, null, null); byte[] buf = new byte[3]; - assertThat(WindowsProcesses.nativeReadStdout(process, buf, -1, 3)).isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 5)).isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 4, 1)).isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 2, -1)).isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStdout(process, buf, Integer.MAX_VALUE, 2)) - .isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 2, Integer.MAX_VALUE)) - .isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, -1, 3)).isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, 0, 5)).isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, 4, 1)).isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, 2, -1)).isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, Integer.MAX_VALUE, 2)) - .isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, 2, Integer.MAX_VALUE)) - .isEqualTo(-1); + assertThat(readStdout(buf, -1, 3)).isEqualTo(-1); + assertThat(readStdout(buf, 0, 5)).isEqualTo(-1); + assertThat(readStdout(buf, 4, 1)).isEqualTo(-1); + assertThat(readStdout(buf, 2, -1)).isEqualTo(-1); + assertThat(readStdout(buf, Integer.MAX_VALUE, 2)).isEqualTo(-1); + assertThat(readStdout(buf, 2, Integer.MAX_VALUE)).isEqualTo(-1); + assertThat(readStderr(buf, -1, 3)).isEqualTo(-1); + assertThat(readStderr(buf, 0, 5)).isEqualTo(-1); + assertThat(readStderr(buf, 4, 1)).isEqualTo(-1); + assertThat(readStderr(buf, 2, -1)).isEqualTo(-1); + assertThat(readStderr(buf, Integer.MAX_VALUE, 2)).isEqualTo(-1); + assertThat(readStderr(buf, 2, Integer.MAX_VALUE)).isEqualTo(-1); assertThat(WindowsProcesses.nativeWriteStdin(process, buf, -1, 3)).isEqualTo(-1); assertThat(WindowsProcesses.nativeWriteStdin(process, buf, 0, 5)).isEqualTo(-1); assertThat(WindowsProcesses.nativeWriteStdin(process, buf, 4, 1)).isEqualTo(-1); @@ -168,7 +178,7 @@ public class WindowsProcessesTest { assertThat(WindowsProcesses.nativeWriteStdin(process, buf, 2, Integer.MAX_VALUE)) .isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 3)).isEqualTo(3); + assertThat(readStdout(buf, 0, 3)).isEqualTo(3); assertThat(new String(buf, UTF8)).isEqualTo("oob"); } @@ -179,9 +189,9 @@ public class WindowsProcessesTest { byte[] output = "abcde".getBytes(UTF8); assertThat(WindowsProcesses.nativeWriteStdin(process, input, 1, 3)).isEqualTo(3); - assertNoError(); - int rv = WindowsProcesses.nativeReadStdout(process, output, 1, 3); - assertNoError(); + assertNoProcessError(); + int rv = readStdout(output, 1, 3); + assertNoProcessError(); assertThat(rv).isEqualTo(3); assertThat(new String(output, UTF8)).isEqualTo("a123e"); @@ -194,24 +204,24 @@ public class WindowsProcessesTest { null, null, null, null); byte[] buf = new byte[4]; - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 4)).isEqualTo(4); + assertThat(readStdout(buf, 0, 4)).isEqualTo(4); assertThat(new String(buf, UTF8)).isEqualTo("out1"); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, 0, 4)).isEqualTo(4); + assertThat(readStderr(buf, 0, 4)).isEqualTo(4); assertThat(new String(buf, UTF8)).isEqualTo("err1"); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, 0, 4)).isEqualTo(4); + assertThat(readStderr(buf, 0, 4)).isEqualTo(4); assertThat(new String(buf, UTF8)).isEqualTo("err2"); - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 4)).isEqualTo(4); + assertThat(readStdout(buf, 0, 4)).isEqualTo(4); assertThat(new String(buf, UTF8)).isEqualTo("out2"); - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 4)).isEqualTo(4); + assertThat(readStdout(buf, 0, 4)).isEqualTo(4); assertThat(new String(buf, UTF8)).isEqualTo("out3"); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, 0, 4)).isEqualTo(4); + assertThat(readStderr(buf, 0, 4)).isEqualTo(4); assertThat(new String(buf, UTF8)).isEqualTo("err3"); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, 0, 4)).isEqualTo(4); + assertThat(readStderr(buf, 0, 4)).isEqualTo(4); assertThat(new String(buf, UTF8)).isEqualTo("err4"); - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 4)).isEqualTo(4); + assertThat(readStdout(buf, 0, 4)).isEqualTo(4); assertThat(new String(buf, UTF8)).isEqualTo("out4"); } @@ -219,18 +229,18 @@ public class WindowsProcessesTest { public void testExecutableNotFound() throws Exception { process = WindowsProcesses.nativeCreateProcess("ThisExecutableDoesNotExist", null, null, null, null); - assertThat(WindowsProcesses.nativeGetLastError(process)) + assertThat(WindowsProcesses.nativeProcessGetLastError(process)) .contains("The system cannot find the file specified."); byte[] buf = new byte[1]; - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 1)).isEqualTo(-1); + assertThat(readStdout(buf, 0, 1)).isEqualTo(-1); } @Test public void testReadingAndWritingAfterTermination() throws Exception { process = WindowsProcesses.nativeCreateProcess("X42", null, null, null, null); byte[] buf = new byte[1]; - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 1)).isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, 0, 1)).isEqualTo(-1); + assertThat(readStdout(buf, 0, 1)).isEqualTo(-1); + assertThat(readStderr(buf, 0, 1)).isEqualTo(-1); assertThat(WindowsProcesses.nativeWriteStdin(process, buf, 0, 1)).isEqualTo(-1); } @@ -239,12 +249,12 @@ public class WindowsProcessesTest { byte[] data = "ONE=one\0TWO=twotwo\0\0".getBytes(UTF8); process = WindowsProcesses.nativeCreateProcess( mockArgs("O$ONE", "O$TWO"), data, null, null, null); - assertNoError(); + assertNoProcessError(); byte[] buf = new byte[3]; - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 3)).isEqualTo(3); + assertThat(readStdout(buf, 0, 3)).isEqualTo(3); assertThat(new String(buf, UTF8)).isEqualTo("one"); buf = new byte[6]; - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 6)).isEqualTo(6); + assertThat(readStdout(buf, 0, 6)).isEqualTo(6); assertThat(new String(buf, UTF8)).isEqualTo("twotwo"); } @@ -252,35 +262,35 @@ public class WindowsProcessesTest { public void testNoZeroInEnvBuffer() throws Exception { byte[] data = "clown".getBytes(UTF8); process = WindowsProcesses.nativeCreateProcess(mockArgs(), data, null, null, null); - assertThat(WindowsProcesses.nativeGetLastError(process)).isNotEmpty(); + assertThat(WindowsProcesses.nativeProcessGetLastError(process)).isNotEmpty(); } @Test public void testMissingFinalDoubleZeroInEnvBuffer() throws Exception { byte[] data = "FOO=bar\0".getBytes(UTF8); process = WindowsProcesses.nativeCreateProcess(mockArgs(), data, null, null, null); - assertThat(WindowsProcesses.nativeGetLastError(process)).isNotEmpty(); + assertThat(WindowsProcesses.nativeProcessGetLastError(process)).isNotEmpty(); } @Test public void testOneByteEnvBuffer() throws Exception { byte[] data = "a".getBytes(UTF8); process = WindowsProcesses.nativeCreateProcess(mockArgs(), data, null, null, null); - assertThat(WindowsProcesses.nativeGetLastError(process)).isNotEmpty(); + assertThat(WindowsProcesses.nativeProcessGetLastError(process)).isNotEmpty(); } @Test public void testOneZeroEnvBuffer() throws Exception { byte[] data = "\0".getBytes(UTF8); process = WindowsProcesses.nativeCreateProcess(mockArgs(), data, null, null, null); - assertThat(WindowsProcesses.nativeGetLastError(process)).isNotEmpty(); + assertThat(WindowsProcesses.nativeProcessGetLastError(process)).isNotEmpty(); } @Test public void testTwoZerosInEnvBuffer() throws Exception { byte[] data = "\0\0".getBytes(UTF8); process = WindowsProcesses.nativeCreateProcess(mockArgs(), data, null, null, null); - assertThat(WindowsProcesses.nativeGetLastError(process)).isEmpty(); + assertThat(WindowsProcesses.nativeProcessGetLastError(process)).isEmpty(); } @Test @@ -291,10 +301,10 @@ public class WindowsProcessesTest { process = WindowsProcesses.nativeCreateProcess(mockArgs("O-one", "E-two"), null, null, stdoutFile, stderrFile); assertThat(process).isGreaterThan(0L); - assertNoError(); + assertNoProcessError(); assertThat(WindowsProcesses.nativeWaitFor(process)).isTrue(); WindowsProcesses.nativeGetExitCode(process); - assertNoError(); + assertNoProcessError(); byte[] stdout = Files.readAllBytes(Paths.get(stdoutFile)); byte[] stderr = Files.readAllBytes(Paths.get(stderrFile)); assertThat(new String(stdout, UTF8)).isEqualTo("one"); @@ -308,10 +318,10 @@ public class WindowsProcessesTest { process = WindowsProcesses.nativeCreateProcess(mockArgs("O-one", "E-two"), null, null, file, file); assertThat(process).isGreaterThan(0L); - assertNoError(); + assertNoProcessError(); assertThat(WindowsProcesses.nativeWaitFor(process)).isTrue(); WindowsProcesses.nativeGetExitCode(process); - assertNoError(); + assertNoProcessError(); byte[] bytes = Files.readAllBytes(Paths.get(file)); assertThat(new String(bytes, UTF8)).isEqualTo("onetwo"); } @@ -323,10 +333,10 @@ public class WindowsProcessesTest { process = WindowsProcesses.nativeCreateProcess(mockArgs("O-one", "E-two"), null, null, stdoutFile, stderrFile); - assertNoError(); + assertNoProcessError(); byte[] buf = new byte[1]; - assertThat(WindowsProcesses.nativeReadStdout(process, buf, 0, 1)).isEqualTo(-1); - assertThat(WindowsProcesses.nativeReadStderr(process, buf, 0, 1)).isEqualTo(-1); + assertThat(readStdout(buf, 0, 1)).isEqualTo(-1); + assertThat(readStderr(buf, 0, 1)).isEqualTo(-1); WindowsProcesses.nativeWaitFor(process); } @@ -341,10 +351,10 @@ public class WindowsProcessesTest { process = WindowsProcesses.nativeCreateProcess(mockArgs("O-out2", "E-err2"), null, null, stdoutFile, stderrFile); - assertNoError(); + assertNoProcessError(); WindowsProcesses.nativeWaitFor(process); WindowsProcesses.nativeGetExitCode(process); - assertNoError(); + assertNoProcessError(); byte[] stdoutBytes = Files.readAllBytes(Paths.get(stdoutFile)); byte[] stderrBytes = Files.readAllBytes(Paths.get(stderrFile)); assertThat(new String(stdoutBytes, UTF8)).isEqualTo("out1out2"); @@ -357,10 +367,10 @@ public class WindowsProcessesTest { new File(dir1).mkdir(); process = WindowsProcesses.nativeCreateProcess(mockArgs("O."), null, dir1, null, null); - assertNoError(); + assertNoProcessError(); byte[] buf = new byte[1024]; // Windows MAX_PATH is 256, but whatever - int len = WindowsProcesses.nativeReadStdout(process, buf, 0, 1024); - assertNoError(); + int len = readStdout(buf, 0, 1024); + assertNoProcessError(); assertThat(new String(buf, 0, len, UTF8).replace("\\", "/")).isEqualTo(dir1); } -- cgit v1.2.3