From 4473bb1a9ec4282aa8497b86580d68e82415df4a Mon Sep 17 00:00:00 2001 From: Philipp Wollermann Date: Tue, 15 Jan 2019 07:54:23 -0800 Subject: [PATCH] Fix a race condition in Bazel's Windows process management. (Second version of this PR that fixes the issues from the last time and refactors the code a bit to make it easier to understand and maintain.) While we did correctly terminate the job object of a process when calling Subprocess.destroy and we did correctly wait for the process itself to exit in Subprocess.waitFor, we didn't wait for its children to exit. This is usually not a big problem, except when they keep open some file handles to files that Bazel immediately tris to delete after killing / waiting for the main process, in which case the files might still be in use. This is the reason why we're seeing errors like this on CI: https://buildkite.com/bazel/bazel-bazel/builds/5774#27afa2c9-9ff1-4224-9df7-ba0253e7e317/193-303 "Caught I/O exception: Cannot delete path (something below $TEST_TMPDIR): Access is denied." This CL modifies the behavior to explicitly terminate the job object in Subprocess.waitFor when the main process has exited and then wait for all transitive subprocesses to exit, before continuing. Due to some interesting semantics, one cannot simply use WaitForSingleObject on the Job handle, instead one has to use an IoCompletionPort that gets a signal when the last job in the group has exited: https://blogs.msdn.microsoft.com/oldnewthing/20130405-00/?p=4743 Closes #6860. PiperOrigin-RevId: 229371044 --- .../build/lib/windows/WindowsSubprocess.java | 59 +- src/main/native/windows/processes-jni.cc | 1045 ++++++++++------- 2 files changed, 633 insertions(+), 471 deletions(-) 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 d99cafe6a7528d..c1562369706213 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 @@ -14,17 +14,17 @@ package com.google.devtools.build.lib.windows; +import com.google.common.base.Throwables; import com.google.devtools.build.lib.shell.Subprocess; import com.google.devtools.build.lib.windows.jni.WindowsProcesses; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.ThreadFactory; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; /** @@ -34,6 +34,11 @@ public class WindowsSubprocess implements Subprocess { // For debugging purposes. private String commandLine; + private static enum WaitResult { + SUCCESS, + TIMEOUT + } + /** * Output stream for writing to the stdin of a Windows process. */ @@ -125,9 +130,9 @@ public Thread newThread(Runnable runnable) { private final OutputStream stdinStream; private final ProcessInputStream stdoutStream; private final ProcessInputStream stderrStream; - private final CountDownLatch waitLatch; + private final Future processFuture; private final long timeoutMillis; - private final AtomicBoolean timedout = new AtomicBoolean(false); + private boolean timedout = false; WindowsSubprocess(long nativeProcess, String commandLine, boolean stdoutRedirected, boolean stderrRedirected, long timeoutMillis) { @@ -140,35 +145,34 @@ public Thread newThread(Runnable runnable) { stderrStream = stderrRedirected ? null : new ProcessInputStream(WindowsProcesses.getStderr(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. - @SuppressWarnings("unused") - Future possiblyIgnoredError = WAITER_POOL.submit(this::waiterThreadFunc); + processFuture = WAITER_POOL.submit(this::waiterThreadFunc); } - private void waiterThreadFunc() { + // Waits for the process to finish. + private WaitResult waiterThreadFunc() { switch (WindowsProcesses.waitFor(nativeProcess, timeoutMillis)) { case 0: // Excellent, process finished in time. - break; + return WaitResult.SUCCESS; case 1: - // Timeout. Terminate the process if we can. - timedout.set(true); - WindowsProcesses.terminate(nativeProcess); - break; + // Timeout. We don't need to call `terminate` here, because waitFor + // automatically terminates the process in case of a timeout. + return WaitResult.TIMEOUT; - case 2: + default: // Error. There isn't a lot we can do -- the process is still alive but - // WaitForMultipleObjects() failed for some odd reason. We'll pretend it terminated and - // log a message to jvm.out . - System.err.println( - "Waiting for process " + WindowsProcesses.getProcessPid(nativeProcess) + " failed"); - break; + // WaitForSingleObject() failed for some odd reason. This should + // basically never happen, but if it does... let's get a stack trace. + String errorMessage = WindowsProcesses.processGetLastError(nativeProcess); + throw new IllegalStateException( + "Waiting for process " + + WindowsProcesses.getProcessPid(nativeProcess) + + " failed: " + + errorMessage); } - - waitLatch.countDown(); } @Override @@ -200,17 +204,24 @@ public synchronized int exitValue() { @Override public boolean finished() { - return waitLatch.getCount() == 0; + return processFuture.isDone(); } @Override public boolean timedout() { - return timedout.get(); + return timedout; } @Override public void waitFor() throws InterruptedException { - waitLatch.await(); + try { + timedout = processFuture.get() == WaitResult.TIMEOUT; + } catch (ExecutionException e) { + Throwables.throwIfUnchecked(e.getCause()); + // This should never happen, because waiterThreadFunc does not throw any + // checked exceptions. + throw new IllegalStateException("Unexpected exception", e); + } } @Override diff --git a/src/main/native/windows/processes-jni.cc b/src/main/native/windows/processes-jni.cc index 4bb214ed8e2d5b..045bd0292ff330 100644 --- a/src/main/native/windows/processes-jni.cc +++ b/src/main/native/windows/processes-jni.cc @@ -28,6 +28,12 @@ #include "src/main/native/windows/jni-util.h" #include "src/main/native/windows/util.h" +// These are the possible return values from the NativeProcess::WaitFor() +// method. +static const int kWaitSuccess = 0; +static const int kWaitTimeout = 1; +static const int kWaitError = 2; + template static std::wstring ToString(const T& e) { std::wstringstream s; @@ -41,48 +47,6 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeGetpid( return GetCurrentProcessId(); } -struct NativeOutputStream { - HANDLE handle_; - std::wstring error_; - std::atomic closed_; - NativeOutputStream() - : handle_(INVALID_HANDLE_VALUE), error_(L""), closed_(false) {} - - void close() { - closed_.store(true); - if (handle_ == INVALID_HANDLE_VALUE) { - return; - } - - // CancelIoEx only cancels I/O operations in the current process. - // https://msdn.microsoft.com/en-us/library/windows/desktop/aa363792(v=vs.85).aspx - // - // Therefore if this process bequested `handle_` to a child process, we - // cannot cancel I/O in the child process. - CancelIoEx(handle_, NULL); - CloseHandle(handle_); - handle_ = INVALID_HANDLE_VALUE; - } -}; - -struct NativeProcess { - HANDLE stdin_; - NativeOutputStream stdout_; - NativeOutputStream stderr_; - HANDLE process_; - HANDLE job_; - DWORD pid_; - std::wstring error_; - - NativeProcess() - : stdin_(INVALID_HANDLE_VALUE), - stdout_(), - stderr_(), - process_(INVALID_HANDLE_VALUE), - job_(INVALID_HANDLE_VALUE), - error_(L"") {} -}; - class JavaByteArray { public: JavaByteArray(JNIEnv* env, jbyteArray java_array) @@ -111,310 +75,637 @@ class JavaByteArray { jbyte* ptr_; }; -static bool NestedJobsSupported() { - OSVERSIONINFOEX version_info; - version_info.dwOSVersionInfoSize = sizeof(version_info); - if (!GetVersionEx(reinterpret_cast(&version_info))) { - return false; - } +class NativeOutputStream { + public: + NativeOutputStream() + : handle_(INVALID_HANDLE_VALUE), error_(L""), closed_(false) {} - return version_info.dwMajorVersion > 6 || - (version_info.dwMajorVersion == 6 && version_info.dwMinorVersion >= 2); -} + void Close() { + closed_.store(true); + if (handle_ == INVALID_HANDLE_VALUE) { + return; + } -// Ensure we can safely cast jlong to void*. -static_assert(sizeof(jlong) == sizeof(void*), - "jlong and void* should be the same size"); + // CancelIoEx only cancels I/O operations in the current process. + // https://msdn.microsoft.com/en-us/library/windows/desktop/aa363792(v=vs.85).aspx + // + // Therefore if this process bequested `handle_` to a child process, we + // cannot cancel I/O in the child process. + CancelIoEx(handle_, NULL); + CloseHandle(handle_); + handle_ = INVALID_HANDLE_VALUE; + } -static_assert(sizeof(jchar) == sizeof(WCHAR), - "jchar and WCHAR should be the same size"); + void SetHandle(HANDLE handle) { handle_ = handle; } -static jlong PtrAsJlong(void* p) { return reinterpret_cast(p); } + jint ReadStream(JNIEnv* env, jbyteArray java_bytes, jint offset, + jint length) { + JavaByteArray bytes(env, java_bytes); + if (offset < 0 || length <= 0 || offset > bytes.size() - length) { + error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"nativeReadStream", L"", + L"Array index out of bounds"); + return -1; + } -extern "C" JNIEXPORT jlong JNICALL -Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProcess( - JNIEnv* env, jclass clazz, jstring java_argv0, jstring java_argv_rest, - jbyteArray java_env, jstring java_cwd, jstring java_stdout_redirect, - jstring java_stderr_redirect, jboolean redirectErrorStream) { - NativeProcess* result = new NativeProcess(); + if (handle_ == INVALID_HANDLE_VALUE || closed_.load()) { + error_ = L""; + return 0; + } - std::wstring argv0; - std::wstring wpath(bazel::windows::GetJavaWpath(env, java_argv0)); - std::wstring error_msg( - bazel::windows::AsExecutablePathForCreateProcess(wpath, &argv0)); - if (!error_msg.empty()) { - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, error_msg); - return PtrAsJlong(result); + DWORD bytes_read; + if (!::ReadFile(handle_, bytes.ptr() + offset, length, &bytes_read, NULL)) { + // Check if either the other end closed the pipe or we did it with + // NativeOutputStream.Close() . In the latter case, we'll get a "system + // call interrupted" error. + if (GetLastError() == ERROR_BROKEN_PIPE || closed_.load()) { + // End of file. + error_ = L""; + bytes_read = 0; + } else { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeReadStream", L"", err_code); + bytes_read = -1; + } + } else { + error_ = L""; + } + + return bytes_read; } - std::wstring commandline = - argv0 + L" " + bazel::windows::GetJavaWstring(env, java_argv_rest); - std::wstring stdout_redirect = bazel::windows::AddUncPrefixMaybe( - bazel::windows::GetJavaWpath(env, java_stdout_redirect)); - std::wstring stderr_redirect = bazel::windows::AddUncPrefixMaybe( - bazel::windows::GetJavaWpath(env, java_stderr_redirect)); - std::wstring cwd; - std::wstring wcwd(bazel::windows::GetJavaWpath(env, java_cwd)); - error_msg = bazel::windows::AsShortPath(wcwd, &cwd); - if (!error_msg.empty()) { - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, error_msg); - return PtrAsJlong(result); + // Return the last error as a human-readable string and clear it. + jstring GetLastErrorAsString(JNIEnv* env) { + jstring result = env->NewString( + reinterpret_cast(error_.c_str()), error_.size()); + error_ = L""; + return result; } - const bool stdout_is_stream = stdout_redirect.empty(); - const bool stderr_is_stream = - redirectErrorStream ? stdout_is_stream : stderr_redirect.empty(); - const bool stderr_same_handle_as_stdout = - redirectErrorStream || - (!stderr_redirect.empty() && - stderr_redirect.size() == stdout_redirect.size() && - _wcsnicmp(stderr_redirect.c_str(), stdout_redirect.c_str(), - stderr_redirect.size()) == 0); - - std::unique_ptr mutable_commandline( - new WCHAR[commandline.size() + 1]); - wcsncpy(mutable_commandline.get(), commandline.c_str(), - commandline.size() + 1); - - SECURITY_ATTRIBUTES sa = {0}; - sa.nLength = sizeof(SECURITY_ATTRIBUTES); - sa.bInheritHandle = TRUE; - - // Standard file handles are closed even if the process was successfully - // created. If this was not so, operations on these file handles would not - // return immediately if the process is terminated. - // Therefore we make these handles auto-closing (by using AutoHandle). - bazel::windows::AutoHandle stdin_process; - bazel::windows::AutoHandle stdout_process; - bazel::windows::AutoHandle stderr_process; - bazel::windows::AutoHandle thread; - PROCESS_INFORMATION process_info = {0}; - JOBOBJECT_EXTENDED_LIMIT_INFORMATION job_info = {0}; - - JavaByteArray env_map(env, java_env); - if (env_map.ptr() != nullptr) { - if (env_map.size() < 4) { - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, - std::wstring(L"the environment must be at least 4 bytes long, was ") + - ToString(env_map.size()) + L" bytes"); - return PtrAsJlong(result); - } else if (env_map.ptr()[env_map.size() - 1] != 0 || - env_map.ptr()[env_map.size() - 2] != 0 || - env_map.ptr()[env_map.size() - 3] != 0 || - env_map.ptr()[env_map.size() - 4] != 0) { - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, - L"environment array must end with 4 null bytes"); - return PtrAsJlong(result); + private: + HANDLE handle_; + std::wstring error_; + std::atomic closed_; +}; + +class NativeProcess { + public: + NativeProcess() + : stdin_(INVALID_HANDLE_VALUE), + stdout_(), + stderr_(), + process_(INVALID_HANDLE_VALUE), + job_(INVALID_HANDLE_VALUE), + ioport_(INVALID_HANDLE_VALUE), + exit_code_(STILL_ACTIVE), + error_(L"") {} + + ~NativeProcess() { + if (stdin_ != INVALID_HANDLE_VALUE) { + CloseHandle(stdin_); } - } - { - HANDLE pipe_read_h, pipe_write_h; - if (!CreatePipe(&pipe_read_h, &pipe_write_h, &sa, 0)) { - DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); - return PtrAsJlong(result); + stdout_.Close(); + stderr_.Close(); + + if (process_ != INVALID_HANDLE_VALUE) { + CloseHandle(process_); + } + + if (job_ != INVALID_HANDLE_VALUE) { + CloseHandle(job_); + } + + if (ioport_ != INVALID_HANDLE_VALUE) { + CloseHandle(ioport_); } - stdin_process = pipe_read_h; - result->stdin_ = pipe_write_h; } - if (!stdout_is_stream) { - result->stdout_.close(); + jboolean Create(JNIEnv* env, jstring java_argv0, jstring java_argv_rest, + jbyteArray java_env, jstring java_cwd, + jstring java_stdout_redirect, jstring java_stderr_redirect, + jboolean redirectErrorStream) { + std::wstring argv0; + std::wstring wpath(bazel::windows::GetJavaWpath(env, java_argv0)); + std::wstring error_msg( + bazel::windows::AsExecutablePathForCreateProcess(wpath, &argv0)); + if (!error_msg.empty()) { + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, error_msg); + return false; + } - stdout_process = CreateFileW( - /* lpFileName */ stdout_redirect.c_str(), - /* dwDesiredAccess */ GENERIC_WRITE, - /* dwShareMode */ 0, - /* lpSecurityAttributes */ &sa, - /* dwCreationDisposition */ OPEN_ALWAYS, - /* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL, - /* hTemplateFile */ NULL); + std::wstring commandline = + argv0 + L" " + bazel::windows::GetJavaWstring(env, java_argv_rest); + std::wstring stdout_redirect = bazel::windows::AddUncPrefixMaybe( + bazel::windows::GetJavaWpath(env, java_stdout_redirect)); + std::wstring stderr_redirect = bazel::windows::AddUncPrefixMaybe( + bazel::windows::GetJavaWpath(env, java_stderr_redirect)); + std::wstring cwd; + std::wstring wcwd(bazel::windows::GetJavaWpath(env, java_cwd)); + error_msg = bazel::windows::AsShortPath(wcwd, &cwd); + if (!error_msg.empty()) { + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, error_msg); + return false; + } - if (!stdout_process.IsValid()) { - DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", stdout_redirect, - err_code); - return PtrAsJlong(result); + const bool stdout_is_stream = stdout_redirect.empty(); + const bool stderr_is_stream = + redirectErrorStream ? stdout_is_stream : stderr_redirect.empty(); + const bool stderr_same_handle_as_stdout = + redirectErrorStream || + (!stderr_redirect.empty() && + stderr_redirect.size() == stdout_redirect.size() && + _wcsnicmp(stderr_redirect.c_str(), stdout_redirect.c_str(), + stderr_redirect.size()) == 0); + + std::unique_ptr mutable_commandline( + new WCHAR[commandline.size() + 1]); + wcsncpy(mutable_commandline.get(), commandline.c_str(), + commandline.size() + 1); + + SECURITY_ATTRIBUTES sa = {0}; + sa.nLength = sizeof(SECURITY_ATTRIBUTES); + sa.bInheritHandle = TRUE; + + // Standard file handles are closed even if the process was successfully + // created. If this was not so, operations on these file handles would not + // return immediately if the process is terminated. + // Therefore we make these handles auto-closing (by using AutoHandle). + bazel::windows::AutoHandle stdin_process; + bazel::windows::AutoHandle stdout_process; + bazel::windows::AutoHandle stderr_process; + bazel::windows::AutoHandle thread; + PROCESS_INFORMATION process_info = {0}; + JOBOBJECT_EXTENDED_LIMIT_INFORMATION job_info = {0}; + + JavaByteArray env_map(env, java_env); + if (env_map.ptr() != nullptr) { + if (env_map.size() < 4) { + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, + std::wstring( + L"the environment must be at least 4 bytes long, was ") + + ToString(env_map.size()) + L" bytes"); + return false; + } else if (env_map.ptr()[env_map.size() - 1] != 0 || + env_map.ptr()[env_map.size() - 2] != 0 || + env_map.ptr()[env_map.size() - 3] != 0 || + env_map.ptr()[env_map.size() - 4] != 0) { + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, + L"environment array must end with 4 null bytes"); + return false; + } + } + + { + HANDLE pipe_read_h, pipe_write_h; + if (!CreatePipe(&pipe_read_h, &pipe_write_h, &sa, 0)) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); + return false; + } + stdin_process = pipe_read_h; + stdin_ = pipe_write_h; + } + + if (!stdout_is_stream) { + stdout_.Close(); + + stdout_process = CreateFileW( + /* lpFileName */ stdout_redirect.c_str(), + /* dwDesiredAccess */ GENERIC_WRITE, + /* dwShareMode */ 0, + /* lpSecurityAttributes */ &sa, + /* dwCreationDisposition */ OPEN_ALWAYS, + /* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL, + /* hTemplateFile */ NULL); + + if (!stdout_process.IsValid()) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"nativeCreateProcess", + stdout_redirect, err_code); + return false; + } + if (!SetFilePointerEx(stdout_process, {0}, NULL, FILE_END)) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"nativeCreateProcess", + stdout_redirect, err_code); + return false; + } + } else { + HANDLE pipe_read_h, pipe_write_h; + if (!CreatePipe(&pipe_read_h, &pipe_write_h, &sa, 0)) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); + return false; + } + stdout_.SetHandle(pipe_read_h); + stdout_process = pipe_write_h; + } + + if (stderr_same_handle_as_stdout) { + HANDLE stdout_process_dup_h; + if (!DuplicateHandle(GetCurrentProcess(), stdout_process, + GetCurrentProcess(), &stdout_process_dup_h, 0, TRUE, + DUPLICATE_SAME_ACCESS)) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); + return false; + } + if (!stderr_is_stream) { + stderr_.Close(); + } + + stderr_process = stdout_process_dup_h; + } else if (!stderr_redirect.empty()) { + stderr_.Close(); + stderr_process = CreateFileW( + /* lpFileName */ stderr_redirect.c_str(), + /* dwDesiredAccess */ GENERIC_WRITE, + /* dwShareMode */ 0, + /* lpSecurityAttributes */ &sa, + /* dwCreationDisposition */ OPEN_ALWAYS, + /* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL, + /* hTemplateFile */ NULL); + + if (!stderr_process.IsValid()) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"nativeCreateProcess", + stderr_redirect, err_code); + return false; + } + if (!SetFilePointerEx(stderr_process, {0}, NULL, FILE_END)) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"nativeCreateProcess", + stderr_redirect, err_code); + return false; + } + } else { + HANDLE pipe_read_h, pipe_write_h; + if (!CreatePipe(&pipe_read_h, &pipe_write_h, &sa, 0)) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); + return false; + } + stderr_.SetHandle(pipe_read_h); + stderr_process = pipe_write_h; } - if (!SetFilePointerEx(stdout_process, {0}, NULL, FILE_END)) { + + // MDSN says that the default for job objects is that breakaway is not + // allowed. Thus, we don't need to do any more setup here. + HANDLE job = CreateJobObject(NULL, NULL); + if (job == NULL) { DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", stdout_redirect, - err_code); - return PtrAsJlong(result); - } - } else { - HANDLE pipe_read_h, pipe_write_h; - if (!CreatePipe(&pipe_read_h, &pipe_write_h, &sa, 0)) { + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); + return false; + } + + job_ = job; + + job_info.BasicLimitInformation.LimitFlags = + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + if (!SetInformationJobObject(job, JobObjectExtendedLimitInformation, + &job_info, sizeof(job_info))) { DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( + error_ = bazel::windows::MakeErrorMessage( WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); - return PtrAsJlong(result); + return false; } - result->stdout_.handle_ = pipe_read_h; - stdout_process = pipe_write_h; - } - if (stderr_same_handle_as_stdout) { - HANDLE stdout_process_dup_h; - if (!DuplicateHandle(GetCurrentProcess(), stdout_process, - GetCurrentProcess(), &stdout_process_dup_h, 0, TRUE, - DUPLICATE_SAME_ACCESS)) { + HANDLE ioport = CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1); + if (ioport == nullptr) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); + return false; + } + ioport_ = ioport; + JOBOBJECT_ASSOCIATE_COMPLETION_PORT port; + port.CompletionKey = job; + port.CompletionPort = ioport; + if (!SetInformationJobObject(job, + JobObjectAssociateCompletionPortInformation, + &port, sizeof(port))) { DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( + error_ = bazel::windows::MakeErrorMessage( WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); - return PtrAsJlong(result); + return false; } - if (!stderr_is_stream) { - result->stderr_.close(); + + std::unique_ptr attr_list; + if (!bazel::windows::AutoAttributeList::Create( + stdin_process, stdout_process, stderr_process, &attr_list, + &error_msg)) { + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", L"", error_msg); + return false; } - stderr_process = stdout_process_dup_h; - } else if (!stderr_redirect.empty()) { - result->stderr_.close(); - stderr_process = CreateFileW( - /* lpFileName */ stderr_redirect.c_str(), - /* dwDesiredAccess */ GENERIC_WRITE, - /* dwShareMode */ 0, - /* lpSecurityAttributes */ &sa, - /* dwCreationDisposition */ OPEN_ALWAYS, - /* dwFlagsAndAttributes */ FILE_ATTRIBUTE_NORMAL, - /* hTemplateFile */ NULL); + // kMaxCmdline value: see lpCommandLine parameter of CreateProcessW. + static constexpr size_t kMaxCmdline = 32767; - if (!stderr_process.IsValid()) { - DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", stderr_redirect, - err_code); - return PtrAsJlong(result); + std::wstring cmd_sample = mutable_commandline.get(); + if (cmd_sample.size() > 200) { + cmd_sample = cmd_sample.substr(0, 195) + L"(...)"; } - if (!SetFilePointerEx(stderr_process, {0}, NULL, FILE_END)) { - DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", stderr_redirect, - err_code); - return PtrAsJlong(result); - } - } else { - HANDLE pipe_read_h, pipe_write_h; - if (!CreatePipe(&pipe_read_h, &pipe_write_h, &sa, 0)) { + if (wcsnlen_s(mutable_commandline.get(), kMaxCmdline) == kMaxCmdline) { + std::wstringstream error_msg; + error_msg << L"command is longer than CreateProcessW's limit (" + << kMaxCmdline << L" characters)"; + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"CreateProcessWithExplicitHandles", + cmd_sample, error_msg.str().c_str()); + return false; + } + + STARTUPINFOEXW info; + attr_list->InitStartupInfoExW(&info); + if (!CreateProcessW( + /* lpApplicationName */ NULL, + /* lpCommandLine */ mutable_commandline.get(), + /* lpProcessAttributes */ NULL, + /* lpThreadAttributes */ NULL, + /* bInheritHandles */ TRUE, + /* dwCreationFlags */ CREATE_NO_WINDOW // Don't create console + // window + | + CREATE_NEW_PROCESS_GROUP // So that Ctrl-Break isn't propagated + | CREATE_SUSPENDED // So that it doesn't start a new job itself + | EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, + /* lpEnvironment */ env_map.ptr(), + /* lpCurrentDirectory */ cwd.empty() ? NULL : cwd.c_str(), + /* lpStartupInfo */ &info.StartupInfo, + /* lpProcessInformation */ &process_info)) { + DWORD err = GetLastError(); + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"CreateProcessW", cmd_sample, err); + return false; + } + + pid_ = process_info.dwProcessId; + process_ = process_info.hProcess; + thread = process_info.hThread; + + if (!AssignProcessToJobObject(job_, process_)) { + BOOL is_in_job = false; + if (IsProcessInJob(process_, NULL, &is_in_job) && is_in_job && + !NestedJobsSupported()) { + // We are on a pre-Windows 8 system and the Bazel is already in a job. + // We can't create nested jobs, so just revert to TerminateProcess() and + // hope for the best. In batch mode, the launcher puts Bazel in a job so + // that will take care of cleanup once the command finishes. + CloseHandle(job_); + job_ = INVALID_HANDLE_VALUE; + CloseHandle(ioport_); + ioport_ = INVALID_HANDLE_VALUE; + } else { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); + return false; + } + } + + // Now that we put the process in a new job object, we can start executing + // it + if (ResumeThread(thread) == -1) { DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( + error_ = bazel::windows::MakeErrorMessage( WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); - return PtrAsJlong(result); + return false; } - result->stderr_.handle_ = pipe_read_h; - stderr_process = pipe_write_h; - } - // MDSN says that the default for job objects is that breakaway is not - // allowed. Thus, we don't need to do any more setup here. - HANDLE job = CreateJobObject(NULL, NULL); - if (job == NULL) { - DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); - return PtrAsJlong(result); + error_ = L""; + return true; } - result->job_ = job; + // Wait for this process to exit (or timeout). + jint WaitFor(jlong timeout_msec) { + DWORD win32_timeout = timeout_msec < 0 ? INFINITE : timeout_msec; + jint result; + switch (WaitForSingleObject(process_, win32_timeout)) { + case WAIT_OBJECT_0: + result = kWaitSuccess; + break; + + case WAIT_TIMEOUT: + result = kWaitTimeout; + break; + + // Any other case is an error and should be reported back to Bazel. + default: + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"NativeProcess:WaitFor", + ToString(pid_), err_code); + return kWaitError; + } + + if (stdin_ != INVALID_HANDLE_VALUE) { + CloseHandle(stdin_); + stdin_ = INVALID_HANDLE_VALUE; + } + + // Ensure that the process is really terminated (if WaitForSingleObject + // above timed out, we have to explicitly kill it) and that it doesn't + // leave behind any subprocesses. + if (!Terminate()) { + return 2; + } + + if (job_ != INVALID_HANDLE_VALUE) { + // Wait for the job object to complete, signalling that all subprocesses + // have exited. + DWORD CompletionCode; + ULONG_PTR CompletionKey; + LPOVERLAPPED Overlapped; + while (GetQueuedCompletionStatus(ioport_, &CompletionCode, &CompletionKey, + &Overlapped, INFINITE) && + !((HANDLE)CompletionKey == job_ && + CompletionCode == JOB_OBJECT_MSG_ACTIVE_PROCESS_ZERO)) { + // Still waiting... + } + + CloseHandle(job_); + job_ = INVALID_HANDLE_VALUE; + + CloseHandle(ioport_); + ioport_ = INVALID_HANDLE_VALUE; + } + + // Fetch and store the exit code in case Bazel asks us for it later, + // because we cannot do this anymore after we closed the handle. + GetExitCode(); - job_info.BasicLimitInformation.LimitFlags = - JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; - if (!SetInformationJobObject(job, JobObjectExtendedLimitInformation, - &job_info, sizeof(job_info))) { - DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); - return PtrAsJlong(result); + if (process_ != INVALID_HANDLE_VALUE) { + CloseHandle(process_); + process_ = INVALID_HANDLE_VALUE; + } + + return result; } - std::unique_ptr attr_list; - if (!bazel::windows::AutoAttributeList::Create(stdin_process, stdout_process, - stderr_process, &attr_list, - &error_msg)) { - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", L"", error_msg); - return PtrAsJlong(result); + // Returns the exit code of the process if it has already exited. If the + // process is still running, returns STILL_ACTIVE (= 259). + jint GetExitCode() { + if (exit_code_ == STILL_ACTIVE) { + if (!GetExitCodeProcess(process_, &exit_code_)) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"NativeProcess::GetExitCode", + ToString(pid_), err_code); + return -1; + } + } + + return exit_code_; } - // kMaxCmdline value: see lpCommandLine parameter of CreateProcessW. - static constexpr size_t kMaxCmdline = 32767; + jint GetPid() { return pid_; } + + jint WriteStdin(JNIEnv* env, jbyteArray java_bytes, jint offset, + jint length) { + JavaByteArray bytes(env, java_bytes); + if (offset < 0 || length <= 0 || offset > bytes.size() - length) { + error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"NativeProcess:WriteStdin", ToString(pid_), + L"Array index out of bounds"); + return -1; + } + + DWORD bytes_written; - std::wstring cmd_sample = mutable_commandline.get(); - if (cmd_sample.size() > 200) { - cmd_sample = cmd_sample.substr(0, 195) + L"(...)"; + if (!::WriteFile(stdin_, bytes.ptr() + offset, length, &bytes_written, + NULL)) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"NativeProcess:WriteStdin", + ToString(pid_), err_code); + return -1; + } + + error_ = L""; + return bytes_written; } - if (wcsnlen_s(mutable_commandline.get(), kMaxCmdline) == kMaxCmdline) { - std::wstringstream error_msg; - error_msg << L"command is longer than CreateProcessW's limit (" - << kMaxCmdline << L" characters)"; - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"CreateProcessWithExplicitHandles", - cmd_sample, error_msg.str().c_str()); - return PtrAsJlong(result); + + NativeOutputStream* GetStdoutStream() { return &stdout_; } + + NativeOutputStream* GetStderrStream() { return &stderr_; } + + // Terminates this process (and subprocesses, if job objects are available). + jboolean Terminate() { + static const UINT exit_code = 130; // 128 + SIGINT, like on Linux + + if (job_ != INVALID_HANDLE_VALUE) { + if (!TerminateJobObject(job_, exit_code)) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"NativeProcess::Terminate", + ToString(pid_), err_code); + return JNI_FALSE; + } + } else if (process_ != INVALID_HANDLE_VALUE) { + if (!TerminateProcess(process_, exit_code)) { + DWORD err_code = GetLastError(); + std::wstring our_error = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"NativeProcess::Terminate", + ToString(pid_), err_code); + + // If the process exited, despite TerminateProcess having failed, we're + // still happy and just ignore the error. It might have been a race + // where the process exited by itself just before we tried to kill it. + // However, if the process is *still* running at this point (evidenced + // by its exit code still being STILL_ACTIVE) then something went + // really unexpectedly wrong and we should report that error. + if (GetExitCode() == STILL_ACTIVE) { + // Restore the error message from TerminateProcess - it will be much + // more helpful for debugging in case something goes wrong here. + error_ = our_error; + return JNI_FALSE; + } + } + + if (WaitForSingleObject(process_, INFINITE) != WAIT_OBJECT_0) { + DWORD err_code = GetLastError(); + error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__, + L"NativeProcess::Terminate", + ToString(pid_), err_code); + return JNI_FALSE; + } + } + + error_ = L""; + return JNI_TRUE; } - STARTUPINFOEXW info; - attr_list->InitStartupInfoExW(&info); - if (!CreateProcessW( - /* lpApplicationName */ NULL, - /* lpCommandLine */ mutable_commandline.get(), - /* lpProcessAttributes */ NULL, - /* lpThreadAttributes */ NULL, - /* bInheritHandles */ TRUE, - /* dwCreationFlags */ CREATE_NO_WINDOW // Don't create console window - | CREATE_NEW_PROCESS_GROUP // So that Ctrl-Break isn't propagated - | CREATE_SUSPENDED // So that it doesn't start a new job itself - | EXTENDED_STARTUPINFO_PRESENT | CREATE_UNICODE_ENVIRONMENT, - /* lpEnvironment */ env_map.ptr(), - /* lpCurrentDirectory */ cwd.empty() ? NULL : cwd.c_str(), - /* lpStartupInfo */ &info.StartupInfo, - /* lpProcessInformation */ &process_info)) { - DWORD err = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"CreateProcessW", cmd_sample, err); - return PtrAsJlong(result); + // Return the last error as a human-readable string and clear it. + jstring GetLastErrorAsString(JNIEnv* env) { + jstring result = env->NewString( + reinterpret_cast(error_.c_str()), error_.size()); + error_ = L""; + return result; } - result->pid_ = process_info.dwProcessId; - result->process_ = process_info.hProcess; - thread = process_info.hThread; - - if (!AssignProcessToJobObject(result->job_, result->process_)) { - BOOL is_in_job = false; - if (IsProcessInJob(result->process_, NULL, &is_in_job) && is_in_job && - !NestedJobsSupported()) { - // We are on a pre-Windows 8 system and the Bazel is already in a job. - // We can't create nested jobs, so just revert to TerminateProcess() and - // hope for the best. In batch mode, the launcher puts Bazel in a job so - // that will take care of cleanup once the command finishes. - CloseHandle(result->job_); - result->job_ = INVALID_HANDLE_VALUE; - } else { - DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); - return PtrAsJlong(result); + private: + bool NestedJobsSupported() { + OSVERSIONINFOEX version_info; + version_info.dwOSVersionInfoSize = sizeof(version_info); + if (!GetVersionEx(reinterpret_cast(&version_info))) { + return false; } - } - // Now that we put the process in a new job object, we can start executing it - if (ResumeThread(thread) == -1) { - DWORD err_code = GetLastError(); - result->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); - return PtrAsJlong(result); + return version_info.dwMajorVersion > 6 || + (version_info.dwMajorVersion == 6 && + version_info.dwMinorVersion >= 2); } - result->error_ = L""; + HANDLE stdin_; + NativeOutputStream stdout_; + NativeOutputStream stderr_; + HANDLE process_; + HANDLE job_; + HANDLE ioport_; + DWORD pid_; + DWORD exit_code_; + std::wstring error_; +}; + +// Ensure we can safely cast jlong to void*. +static_assert(sizeof(jlong) == sizeof(void*), + "jlong and void* should be the same size"); + +static_assert(sizeof(jchar) == sizeof(WCHAR), + "jchar and WCHAR should be the same size"); + +static jlong PtrAsJlong(void* p) { return reinterpret_cast(p); } + +extern "C" JNIEXPORT jlong JNICALL +Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProcess( + JNIEnv* env, jclass clazz, jstring java_argv0, jstring java_argv_rest, + jbyteArray java_env, jstring java_cwd, jstring java_stdout_redirect, + jstring java_stderr_redirect, jboolean redirectErrorStream) { + NativeProcess* result = new NativeProcess(); + // TODO(philwo) The `Create` method returns false in case of an error. But + // there seems to be no good way to signal an error at this point to Bazel. + // The way the code currently works is that the Java code explicitly calls + // nativeProcessGetLastError(), so it's OK, but it would be nice if we + // could just throw an exception here. + result->Create(env, java_argv0, java_argv_rest, java_env, java_cwd, + java_stdout_redirect, java_stderr_redirect, + redirectErrorStream); return PtrAsJlong(result); } @@ -423,42 +714,21 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeWriteStdin JNIEnv* env, jclass clazz, jlong process_long, jbyteArray java_bytes, jint offset, jint length) { NativeProcess* process = reinterpret_cast(process_long); - - JavaByteArray bytes(env, java_bytes); - if (offset < 0 || length <= 0 || offset > bytes.size() - length) { - process->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeWriteStdin", ToString(process->pid_), - L"Array index out of bounds"); - return -1; - } - - DWORD bytes_written; - - if (!::WriteFile(process->stdin_, bytes.ptr() + offset, length, - &bytes_written, NULL)) { - DWORD err_code = GetLastError(); - process->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeWriteStdin", ToString(process->pid_), - err_code); - bytes_written = -1; - } - - process->error_ = L""; - return bytes_written; + return process->WriteStdin(env, java_bytes, offset, length); } extern "C" JNIEXPORT jlong JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeGetStdout( JNIEnv* env, jclass clazz, jlong process_long) { NativeProcess* process = reinterpret_cast(process_long); - return PtrAsJlong(&process->stdout_); + return PtrAsJlong(process->GetStdoutStream()); } extern "C" JNIEXPORT jlong JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeGetStderr( JNIEnv* env, jclass clazz, jlong process_long) { NativeProcess* process = reinterpret_cast(process_long); - return PtrAsJlong(&process->stderr_); + return PtrAsJlong(process->GetStderrStream()); } extern "C" JNIEXPORT jint JNICALL @@ -467,57 +737,14 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeReadStream jint offset, jint length) { NativeOutputStream* stream = reinterpret_cast(stream_long); - - JavaByteArray bytes(env, java_bytes); - if (offset < 0 || length <= 0 || offset > bytes.size() - length) { - stream->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeReadStream", L"", - L"Array index out of bounds"); - return -1; - } - - if (stream->handle_ == INVALID_HANDLE_VALUE || stream->closed_.load()) { - stream->error_ = L""; - return 0; - } - - DWORD bytes_read; - if (!::ReadFile(stream->handle_, bytes.ptr() + offset, length, &bytes_read, - NULL)) { - // Check if either the other end closed the pipe or we did it with - // NativeOutputStream.close() . In the latter case, we'll get a "system - // call interrupted" error. - if (GetLastError() == ERROR_BROKEN_PIPE || stream->closed_.load()) { - // End of file. - stream->error_ = L""; - bytes_read = 0; - } else { - DWORD err_code = GetLastError(); - stream->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeReadStream", L"", err_code); - bytes_read = -1; - } - } else { - stream->error_ = L""; - } - - return bytes_read; + return stream->ReadStream(env, java_bytes, offset, length); } extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeGetExitCode( JNIEnv* env, jclass clazz, jlong process_long) { NativeProcess* process = reinterpret_cast(process_long); - DWORD exit_code; - if (!GetExitCodeProcess(process->process_, &exit_code)) { - DWORD err_code = GetLastError(); - process->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeGetExitCode", ToString(process->pid_), - err_code); - return -1; - } - - return exit_code; + return process->GetExitCode(); } // return values: @@ -528,95 +755,27 @@ extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeWaitFor( JNIEnv* env, jclass clazz, jlong process_long, jlong java_timeout) { NativeProcess* process = reinterpret_cast(process_long); - HANDLE handles[1] = {process->process_}; - DWORD win32_timeout = java_timeout < 0 ? INFINITE : java_timeout; - jint result; - switch (WaitForMultipleObjects(1, handles, FALSE, win32_timeout)) { - case 0: - result = 0; - break; - - case WAIT_TIMEOUT: - result = 1; - break; - - case WAIT_FAILED: - result = 2; - break; - - default: - DWORD err_code = GetLastError(); - process->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeWaitFor", ToString(process->pid_), - err_code); - break; - } - - if (process->stdin_ != INVALID_HANDLE_VALUE) { - CloseHandle(process->stdin_); - process->stdin_ = INVALID_HANDLE_VALUE; - } - - return result; + return process->WaitFor(java_timeout); } extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeGetProcessPid( JNIEnv* env, jclass clazz, jlong process_long) { NativeProcess* process = reinterpret_cast(process_long); - process->error_ = L""; - return GetProcessId(process->process_); // MSDN says that this cannot fail + return process->GetPid(); } extern "C" JNIEXPORT jboolean JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeTerminate( JNIEnv* env, jclass clazz, jlong process_long) { - static const UINT exit_code = 130; // 128 + SIGINT, like on Linux NativeProcess* process = reinterpret_cast(process_long); - - if (process->job_ != INVALID_HANDLE_VALUE) { - if (!TerminateJobObject(process->job_, exit_code)) { - DWORD err_code = GetLastError(); - process->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeTerminate", ToString(process->pid_), - err_code); - return JNI_FALSE; - } - } else if (process->process_ != INVALID_HANDLE_VALUE) { - if (!TerminateProcess(process->process_, exit_code)) { - DWORD err_code = GetLastError(); - process->error_ = bazel::windows::MakeErrorMessage( - WSTR(__FILE__), __LINE__, L"nativeTerminate", ToString(process->pid_), - err_code); - return JNI_FALSE; - } - } - - process->error_ = L""; - return JNI_TRUE; + return process->Terminate(); } extern "C" JNIEXPORT void JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeDeleteProcess( JNIEnv* env, jclass clazz, jlong process_long) { - NativeProcess* process = reinterpret_cast(process_long); - - if (process->stdin_ != INVALID_HANDLE_VALUE) { - CloseHandle(process->stdin_); - } - - process->stdout_.close(); - process->stderr_.close(); - - if (process->process_ != INVALID_HANDLE_VALUE) { - CloseHandle(process->process_); - } - - if (process->job_ != INVALID_HANDLE_VALUE) { - CloseHandle(process->job_); - } - - delete process; + delete reinterpret_cast(process_long); } extern "C" JNIEXPORT void JNICALL @@ -624,18 +783,14 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCloseStrea JNIEnv* env, jclass clazz, jlong stream_long) { NativeOutputStream* stream = reinterpret_cast(stream_long); - stream->close(); + stream->Close(); } extern "C" JNIEXPORT jstring JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeProcessGetLastError( JNIEnv* env, jclass clazz, jlong process_long) { NativeProcess* process = reinterpret_cast(process_long); - jstring result = - env->NewString(reinterpret_cast(process->error_.c_str()), - process->error_.size()); - process->error_ = L""; - return result; + return process->GetLastErrorAsString(env); } extern "C" JNIEXPORT jstring JNICALL @@ -643,9 +798,5 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeStreamGetL JNIEnv* env, jclass clazz, jlong stream_long) { NativeOutputStream* stream = reinterpret_cast(stream_long); - jstring result = - env->NewString(reinterpret_cast(stream->error_.c_str()), - stream->error_.size()); - stream->error_ = L""; - return result; + return stream->GetLastErrorAsString(env); }