diff --git a/client/client_test.gyp b/client/client_test.gyp index dec8f603..ffafcdbd 100644 --- a/client/client_test.gyp +++ b/client/client_test.gyp @@ -22,6 +22,7 @@ 'type': 'executable', 'dependencies': [ 'client.gyp:crashpad_client', + '../compat/compat.gyp:crashpad_compat', '../handler/handler.gyp:crashpad_handler', '../test/test.gyp:crashpad_gmock_main', '../test/test.gyp:crashpad_test', diff --git a/client/crashpad_client_win_test.cc b/client/crashpad_client_win_test.cc index 47e7add9..8441e6a3 100644 --- a/client/crashpad_client_win_test.cc +++ b/client/crashpad_client_win_test.cc @@ -14,25 +14,187 @@ #include "client/crashpad_client.h" +#include + +#include + #include "base/files/file_path.h" #include "base/macros.h" +#include "base/memory/ptr_util.h" +#include "base/logging.h" #include "gtest/gtest.h" +#include "test/errors.h" #include "test/paths.h" #include "test/scoped_temp_dir.h" +#include "util/win/process_info.h" #include "test/win/win_multiprocess.h" +#include "util/win/scoped_handle.h" #include "util/win/termination_codes.h" namespace crashpad { namespace test { namespace { -void StartAndUseHandler() { - ScopedTempDir temp_dir; +class ScopedEnvironmentVariable { + public: + explicit ScopedEnvironmentVariable(const wchar_t* name); + ~ScopedEnvironmentVariable(); + + std::wstring GetValue() const; + + // Sets this environment variable to |new_value|. If |new_value| is nullptr + // this environment variable will be undefined. + void SetValue(const wchar_t* new_value) const; + + private: + std::wstring GetValueImpl(bool* is_defined) const; + + std::wstring original_value_; + const wchar_t* name_; + bool was_defined_; + + DISALLOW_COPY_AND_ASSIGN(ScopedEnvironmentVariable); +}; + +ScopedEnvironmentVariable::ScopedEnvironmentVariable(const wchar_t* name) + : name_(name) { + original_value_ = GetValueImpl(&was_defined_); +} + +ScopedEnvironmentVariable::~ScopedEnvironmentVariable() { + if (was_defined_) + SetValue(original_value_.data()); + else + SetValue(nullptr); +} + +std::wstring ScopedEnvironmentVariable::GetValue() const { + bool dummy; + return GetValueImpl(&dummy); +} + +std::wstring ScopedEnvironmentVariable::GetValueImpl(bool* is_defined) const { + // The length returned is inclusive of the terminating zero, except + // if the variable doesn't exist, in which case the return value is zero. + DWORD len = GetEnvironmentVariable(name_, nullptr, 0); + if (len == 0) { + *is_defined = false; + return L""; + } + + *is_defined = true; + + std::wstring ret; + ret.resize(len); + // The length returned on success is exclusive of the terminating zero. + len = GetEnvironmentVariable(name_, &ret[0], len); + ret.resize(len); + + return ret; +} + +void ScopedEnvironmentVariable::SetValue(const wchar_t* new_value) const { + SetEnvironmentVariable(name_, new_value); +} + +// Returns the process IDs of all processes that have |parent_pid| as +// parent process ID. +std::vector GetPotentialChildProcessesOf(pid_t parent_pid) { + ScopedFileHANDLE snapshot(CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0)); + if (!snapshot.is_valid()) { + ADD_FAILURE() << ErrorMessage("CreateToolhelp32Snapshot"); + return std::vector(); + } + + PROCESSENTRY32 entry = {sizeof(entry)}; + if (!Process32First(snapshot.get(), &entry)) { + ADD_FAILURE() << ErrorMessage("Process32First"); + return std::vector(); + } + + std::vector child_pids; + do { + if (entry.th32ParentProcessID == parent_pid) + child_pids.push_back(entry.th32ProcessID); + } while (Process32Next(snapshot.get(), &entry)); + + return child_pids; +} + +ULARGE_INTEGER GetProcessCreationTime(HANDLE process) { + ULARGE_INTEGER ret = {}; + FILETIME creation_time; + FILETIME dummy; + if (GetProcessTimes(process, &creation_time, &dummy, &dummy, &dummy)) { + ret.LowPart = creation_time.dwLowDateTime; + ret.HighPart = creation_time.dwHighDateTime; + } else { + ADD_FAILURE() << ErrorMessage("GetProcessTimes"); + } + + return ret; +} + +// Waits for the processes directly created by |parent| - and specifically not +// their offspring. For this to work without race, |parent| has to be suspended +// or have exited. +void WaitForAllChildProcessesOf(HANDLE parent) { + pid_t parent_pid = GetProcessId(parent); + std::vector child_pids = GetPotentialChildProcessesOf(parent_pid); + + ULARGE_INTEGER parent_creationtime = GetProcessCreationTime(parent); + for (pid_t child_pid : child_pids) { + // Try and open the process. This may fail for reasons such as: + // 1. The process isn't |parent|'s child process, but rather a + // higher-privilege sub-process of an earlier process that had + // |parent|'s PID. + // 2. The process no longer exists, e.g. it exited after enumeration. + ScopedKernelHANDLE child_process( + OpenProcess(PROCESS_VM_READ | PROCESS_QUERY_INFORMATION | SYNCHRONIZE, + false, + child_pid)); + if (!child_process.is_valid()) + continue; + + // Check that the child now has the right parent PID, as its PID may have + // been reused after the enumeration above. + ProcessInfo child_info; + if (!child_info.Initialize(child_process.get())) { + // This can happen if child_process has exited after the handle is opened. + LOG(ERROR) << "ProcessInfo::Initialize, pid: " << child_pid; + continue; + } + + if (parent_pid != child_info.ParentProcessID()) { + // The child's process ID was reused after enumeration. + continue; + } + + // We successfully opened |child_process| and it has |parent|'s PID for + // parent process ID. However, this could still be a sub-process of another + // process that earlier had |parent|'s PID. To make sure, check that + // |child_process| was created after |parent_process|. + ULARGE_INTEGER process_creationtime = + GetProcessCreationTime(child_process.get()); + if (process_creationtime.QuadPart < parent_creationtime.QuadPart) + continue; + + DWORD err = WaitForSingleObject(child_process.get(), INFINITE); + if (err == WAIT_FAILED) { + ADD_FAILURE() << ErrorMessage("WaitForSingleObject"); + } else if (err != WAIT_OBJECT_0) { + ADD_FAILURE() << "WaitForSingleObject returned " << err; + } + } +} + +void StartAndUseHandler(const base::FilePath& temp_dir) { base::FilePath handler_path = Paths::Executable().DirName().Append( FILE_PATH_LITERAL("crashpad_handler.com")); + CrashpadClient client; ASSERT_TRUE(client.StartHandler(handler_path, - temp_dir.path(), + temp_dir, base::FilePath(), "", std::map(), @@ -42,9 +204,35 @@ void StartAndUseHandler() { ASSERT_TRUE(client.WaitForHandlerStart(INFINITE)); } -class StartWithInvalidHandles final : public WinMultiprocess { +// Name of the environment variable used to communicate the name of the +// temp directory from parent to child process. +constexpr wchar_t kTempDirEnvName[] = L"CRASHPAD_TEST_TEMP_DIR"; + +class WinMultiprocessWithTempDir : public WinMultiprocess { public: - StartWithInvalidHandles() : WinMultiprocess() {} + WinMultiprocessWithTempDir() + : WinMultiprocess(), temp_dir_env_(kTempDirEnvName) {} + + void WinMultiprocessParentBeforeChild() override { + temp_dir_ = base::WrapUnique(new ScopedTempDir); + temp_dir_env_.SetValue(temp_dir_->path().value().c_str()); + } + + void WinMultiprocessParentAfterChild(HANDLE child) override { + WaitForAllChildProcessesOf(child); + temp_dir_.reset(); + } + + protected: + std::unique_ptr temp_dir_; + ScopedEnvironmentVariable temp_dir_env_; + + DISALLOW_COPY_AND_ASSIGN(WinMultiprocessWithTempDir); +}; + +class StartWithInvalidHandles final : public WinMultiprocessWithTempDir { + public: + StartWithInvalidHandles() : WinMultiprocessWithTempDir() {} ~StartWithInvalidHandles() {} private: @@ -56,7 +244,7 @@ class StartWithInvalidHandles final : public WinMultiprocess { SetStdHandle(STD_OUTPUT_HANDLE, INVALID_HANDLE_VALUE); SetStdHandle(STD_ERROR_HANDLE, INVALID_HANDLE_VALUE); - StartAndUseHandler(); + StartAndUseHandler(base::FilePath(temp_dir_env_.GetValue())); SetStdHandle(STD_OUTPUT_HANDLE, original_stdout); SetStdHandle(STD_ERROR_HANDLE, original_stderr); @@ -67,9 +255,9 @@ TEST(CrashpadClient, StartWithInvalidHandles) { WinMultiprocess::Run(); } -class StartWithSameStdoutStderr final : public WinMultiprocess { +class StartWithSameStdoutStderr final : public WinMultiprocessWithTempDir { public: - StartWithSameStdoutStderr() : WinMultiprocess() {} + StartWithSameStdoutStderr() : WinMultiprocessWithTempDir() {} ~StartWithSameStdoutStderr() {} private: @@ -80,7 +268,7 @@ class StartWithSameStdoutStderr final : public WinMultiprocess { HANDLE original_stderr = GetStdHandle(STD_ERROR_HANDLE); SetStdHandle(STD_OUTPUT_HANDLE, original_stderr); - StartAndUseHandler(); + StartAndUseHandler(base::FilePath(temp_dir_env_.GetValue())); SetStdHandle(STD_OUTPUT_HANDLE, original_stdout); } diff --git a/test/win/win_multiprocess.h b/test/win/win_multiprocess.h index 663c7594..66347010 100644 --- a/test/win/win_multiprocess.h +++ b/test/win/win_multiprocess.h @@ -43,12 +43,16 @@ class WinMultiprocess { ASSERT_NO_FATAL_FAILURE( WinChildProcess::EntryPoint>()); // If WinChildProcess::EntryPoint returns, we are in the parent process. + T parent_process; + WinMultiprocess* parent_multiprocess = &parent_process; + + parent_multiprocess->WinMultiprocessParentBeforeChild(); + std::unique_ptr child_handles = WinChildProcess::Launch(); ASSERT_TRUE(child_handles.get()); - T parent_process; parent_process.child_handles_ = child_handles.get(); - static_cast(&parent_process)->WinMultiprocessParent(); + parent_multiprocess->WinMultiprocessParent(); // Close our side of the handles now that we're done. The child can // use this to know when it's safe to complete. @@ -62,6 +66,9 @@ class WinMultiprocess { DWORD exit_code; ASSERT_TRUE(GetExitCodeProcess(child_handles->process.get(), &exit_code)); ASSERT_EQ(parent_process.exit_code_, exit_code); + + parent_multiprocess->WinMultiprocessParentAfterChild( + child_handles->process.get()); } protected: @@ -162,6 +169,20 @@ class WinMultiprocess { //! Subclasses must implement this method to define how the parent operates. virtual void WinMultiprocessParent() = 0; + //! \brief The optional routine run in parent before the child is spawned. + //! + //! Subclasses may implement this method to prepare the environment for + //! the child process. + virtual void WinMultiprocessParentBeforeChild() {} + + //! \brief The optional routine run in parent after the child exits. + //! + //! Subclasses may implement this method to clean up the environment after + //! the child process has exited. + //! + //! \param[in] child A handle to the exited child process. + virtual void WinMultiprocessParentAfterChild(HANDLE child) {} + //! \brief The subclass-provided child routine. //! //! Test failures should be reported via gtest: `EXPECT_*()`, `ASSERT_*()`,