From 71886678d4b6a71ce9bf01a768d7615bf394ce4d Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Wed, 25 Oct 2017 17:48:07 +0200 Subject: Windows,JNI,logging: improve error messages In this commit: - introduce the MakeErrorMessage function, which creates a structured error message with file and line information of the error's origin - update all error messages in the Windows JNI library - simplify GetLastErrorMessage to just convert an error code to string, without prepending a cause Change-Id: Ia8162bfdaee37d4b7ccb3a46d6c8a861b0a1bd94 PiperOrigin-RevId: 173402968 --- src/main/native/windows/processes-jni.cc | 123 ++++++++++++++++++++----------- 1 file changed, 82 insertions(+), 41 deletions(-) (limited to 'src/main/native/windows/processes-jni.cc') diff --git a/src/main/native/windows/processes-jni.cc b/src/main/native/windows/processes-jni.cc index 78ba8d2b46..27d8b4548d 100644 --- a/src/main/native/windows/processes-jni.cc +++ b/src/main/native/windows/processes-jni.cc @@ -18,6 +18,7 @@ #include #include +#include #include #include // static_assert @@ -25,6 +26,13 @@ #include "src/main/native/windows/jni-util.h" #include "src/main/native/windows/util.h" +template +static std::wstring ToString(const T& e) { + std::wstringstream s; + s << e; + return s.str(); +} + extern "C" JNIEXPORT jint JNICALL Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeGetpid( JNIEnv* env, jclass clazz) { @@ -125,12 +133,6 @@ static_assert(sizeof(jchar) == sizeof(WCHAR), static jlong PtrAsJlong(void* p) { return reinterpret_cast(p); } -static std::wstring AsExecutableForCreateProcess(JNIEnv* env, jstring path, - std::wstring* result) { - return bazel::windows::AsExecutablePathForCreateProcess( - bazel::windows::GetJavaWstring(env, path), result); -} - // The following CreateProcessWithExplicitHandles function is from oldnewthing. // https://blogs.msdn.microsoft.com/oldnewthing/20111216-00/?p=8873 // We need it to prevent unintended handles being inherited by child process, @@ -206,9 +208,12 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc NativeProcess* result = new NativeProcess(); std::wstring argv0; - std::wstring error_msg(AsExecutableForCreateProcess(env, java_argv0, &argv0)); + std::wstring wpath(bazel::windows::GetJavaWstring(env, java_argv0)); + std::wstring error_msg( + bazel::windows::AsExecutablePathForCreateProcess(wpath, &argv0)); if (!error_msg.empty()) { - result->error_ = error_msg; + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, error_msg); return PtrAsJlong(result); } @@ -219,10 +224,11 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc std::wstring stderr_redirect = AddUncPrefixMaybe( bazel::windows::GetJavaWstring(env, java_stderr_redirect)); std::wstring cwd; - error_msg = bazel::windows::AsShortPath( - bazel::windows::GetJavaWstring(env, java_cwd), &cwd); + std::wstring wcwd(bazel::windows::GetJavaWstring(env, java_cwd)); + error_msg = bazel::windows::AsShortPath(wcwd, &cwd); if (!error_msg.empty()) { - result->error_ = error_msg; + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, error_msg); return PtrAsJlong(result); } @@ -250,18 +256,24 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc JavaByteArray env_map(env, java_env); if (env_map.ptr() != nullptr) { if (env_map.size() < 2) { - result->error_ = L"The environment must be at least two bytes long"; + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, + L"the environment must be at least two bytes long"); return PtrAsJlong(result); } else if (env_map.ptr()[env_map.size() - 1] != 0 || env_map.ptr()[env_map.size() - 2] != 0) { - result->error_ = L"Environment array must end with two null bytes"; + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, + L"environment array must end with two null bytes"); return PtrAsJlong(result); } } HANDLE temp_stdin_handle = INVALID_HANDLE_VALUE; if (!CreatePipe(&temp_stdin_handle, &result->stdin_, &sa, 0)) { - result->error_ = bazel::windows::GetLastErrorString(L"CreatePipe(stdin)"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); CloseHandle(temp_stdin_handle); return PtrAsJlong(result); } @@ -280,15 +292,17 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc /* hTemplateFile */ NULL); if (!stdout_process.IsValid()) { - result->error_ = - bazel::windows::GetLastErrorString(L"CreateFile(stdout)"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } } else { HANDLE temp_stdout_handle = INVALID_HANDLE_VALUE; if (!CreatePipe(&result->stdout_.handle_, &temp_stdout_handle, &sa, 0)) { - result->error_ = - bazel::windows::GetLastErrorString(L"CreatePipe(stdout)"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); CloseHandle(temp_stdout_handle); return PtrAsJlong(result); } @@ -326,8 +340,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc /* hTemplateFile */ NULL); if (stderr_handle == INVALID_HANDLE_VALUE) { - result->error_ = - bazel::windows::GetLastErrorString(L"CreateFile(stderr)"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } // stderr_process != stdout_process, so set its handle, so the AutoHandle @@ -336,8 +351,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc } } else { if (!CreatePipe(&result->stderr_.handle_, &stderr_handle, &sa, 0)) { - result->error_ = - bazel::windows::GetLastErrorString(L"CreatePipe(stderr)"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } stderr_process = stderr_handle; @@ -347,7 +363,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc // allowed. Thus, we don't need to do any more setup here. HANDLE job = CreateJobObject(NULL, NULL); if (job == NULL) { - result->error_ = bazel::windows::GetLastErrorString(L"CreateJobObject()"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } @@ -357,8 +375,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; if (!SetInformationJobObject(job, JobObjectExtendedLimitInformation, &job_info, sizeof(job_info))) { - result->error_ = - bazel::windows::GetLastErrorString(L"SetInformationJobObject()"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } @@ -386,7 +405,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc /* rgHandlesToInherit */ handlesToInherit); if (!ok) { - result->error_ = bazel::windows::GetLastErrorString(L"CreateProcess()"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } @@ -405,15 +426,18 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeCreateProc CloseHandle(result->job_); result->job_ = INVALID_HANDLE_VALUE; } else { - result->error_ = - bazel::windows::GetLastErrorString(L"AssignProcessToJobObject()"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } } // Now that we put the process in a new job object, we can start executing it if (ResumeThread(thread) == -1) { - result->error_ = bazel::windows::GetLastErrorString(L"ResumeThread()"); + DWORD err_code = GetLastError(); + result->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code); return PtrAsJlong(result); } @@ -429,7 +453,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeWriteStdin JavaByteArray bytes(env, java_bytes); if (offset < 0 || length <= 0 || offset > bytes.size() - length) { - process->error_ = L"Array index out of bounds"; + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeWriteStdin", ToString(process->pid_), + L"Array index out of bounds"); return -1; } @@ -437,7 +463,10 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeWriteStdin if (!::WriteFile(process->stdin_, bytes.ptr() + offset, length, &bytes_written, NULL)) { - process->error_ = bazel::windows::GetLastErrorString(L"WriteFile()"); + DWORD err_code = GetLastError(); + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeWriteStdin", ToString(process->pid_), + err_code); bytes_written = -1; } @@ -468,7 +497,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeReadStream JavaByteArray bytes(env, java_bytes); if (offset < 0 || length <= 0 || offset > bytes.size() - length) { - stream->error_ = L"Array index out of bounds"; + stream->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeReadStream", L"", + L"Array index out of bounds"); return -1; } @@ -488,7 +519,9 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeReadStream stream->error_ = L""; bytes_read = 0; } else { - stream->error_ = bazel::windows::GetLastErrorString(L"ReadFile()"); + DWORD err_code = GetLastError(); + stream->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeReadStream", L"", err_code); bytes_read = -1; } } else { @@ -504,8 +537,10 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeGetExitCod NativeProcess* process = reinterpret_cast(process_long); DWORD exit_code; if (!GetExitCodeProcess(process->process_, &exit_code)) { - process->error_ = - bazel::windows::GetLastErrorString(L"GetExitCodeProcess()"); + DWORD err_code = GetLastError(); + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeGetExitCode", ToString(process->pid_), + err_code); return -1; } @@ -537,8 +572,10 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeWaitFor( break; default: - process->error_ = L"WaitForMultipleObjects() returned unknown result"; - result = 2; + DWORD err_code = GetLastError(); + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeWaitFor", ToString(process->pid_), + err_code); break; } @@ -566,14 +603,18 @@ Java_com_google_devtools_build_lib_windows_jni_WindowsProcesses_nativeTerminate( if (process->job_ != INVALID_HANDLE_VALUE) { if (!TerminateJobObject(process->job_, exit_code)) { - process->error_ = - bazel::windows::GetLastErrorString(L"TerminateJobObject()"); + 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)) { - process->error_ = - bazel::windows::GetLastErrorString(L"TerminateProcess()"); + DWORD err_code = GetLastError(); + process->error_ = bazel::windows::MakeErrorMessage( + WSTR(__FILE__), __LINE__, L"nativeTerminate", ToString(process->pid_), + err_code); return JNI_FALSE; } } -- cgit v1.2.3