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); }