From ab9c03f8827b667178f90dc23c13080c845a715f Mon Sep 17 00:00:00 2001 From: Sigurdur Asgeirsson Date: Fri, 7 Apr 2017 10:46:41 -0400 Subject: [PATCH] win: Promote WinMultiProcessWithTempDir to test/win for reuse. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: crashpad:167 Change-Id: I80a4a58246d479bceb7154f270f34380a65ebf6d Reviewed-on: https://chromium-review.googlesource.com/470110 Commit-Queue: Sigurður Ásgeirsson Reviewed-by: Mark Mentovai --- client/crashpad_client_win_test.cc | 194 +-------------------- test/test.gyp | 2 + test/win/win_multiprocess_with_temp_dir.cc | 188 ++++++++++++++++++++ test/win/win_multiprocess_with_temp_dir.h | 80 +++++++++ 4 files changed, 276 insertions(+), 188 deletions(-) create mode 100644 test/win/win_multiprocess_with_temp_dir.cc create mode 100644 test/win/win_multiprocess_with_temp_dir.h diff --git a/client/crashpad_client_win_test.cc b/client/crashpad_client_win_test.cc index 743e930d..7e5093f2 100644 --- a/client/crashpad_client_win_test.cc +++ b/client/crashpad_client_win_test.cc @@ -14,8 +14,6 @@ #include "client/crashpad_client.h" -#include - #include #include "base/files/file_path.h" @@ -23,11 +21,10 @@ #include "base/memory/ptr_util.h" #include "base/logging.h" #include "gtest/gtest.h" -#include "test/errors.h" -#include "test/scoped_temp_dir.h" #include "test/test_paths.h" +#include "test/scoped_temp_dir.h" #include "test/win/win_multiprocess.h" -#include "util/win/process_info.h" +#include "test/win/win_multiprocess_with_temp_dir.h" #include "util/win/scoped_handle.h" #include "util/win/termination_codes.h" @@ -35,159 +32,6 @@ namespace crashpad { namespace test { namespace { -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 = TestPaths::Executable().DirName().Append( FILE_PATH_LITERAL("crashpad_handler.com")); @@ -204,32 +48,6 @@ void StartAndUseHandler(const base::FilePath& temp_dir) { ASSERT_TRUE(client.WaitForHandlerStart(INFINITE)); } -// 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: - 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() {} @@ -244,7 +62,7 @@ class StartWithInvalidHandles final : public WinMultiprocessWithTempDir { SetStdHandle(STD_OUTPUT_HANDLE, INVALID_HANDLE_VALUE); SetStdHandle(STD_ERROR_HANDLE, INVALID_HANDLE_VALUE); - StartAndUseHandler(base::FilePath(temp_dir_env_.GetValue())); + StartAndUseHandler(GetTempDirPath()); SetStdHandle(STD_OUTPUT_HANDLE, original_stdout); SetStdHandle(STD_ERROR_HANDLE, original_stderr); @@ -252,7 +70,7 @@ class StartWithInvalidHandles final : public WinMultiprocessWithTempDir { }; TEST(CrashpadClient, StartWithInvalidHandles) { - WinMultiprocess::Run(); + WinMultiprocessWithTempDir::Run(); } class StartWithSameStdoutStderr final : public WinMultiprocessWithTempDir { @@ -268,14 +86,14 @@ class StartWithSameStdoutStderr final : public WinMultiprocessWithTempDir { HANDLE original_stderr = GetStdHandle(STD_ERROR_HANDLE); SetStdHandle(STD_OUTPUT_HANDLE, original_stderr); - StartAndUseHandler(base::FilePath(temp_dir_env_.GetValue())); + StartAndUseHandler(GetTempDirPath()); SetStdHandle(STD_OUTPUT_HANDLE, original_stdout); } }; TEST(CrashpadClient, StartWithSameStdoutStderr) { - WinMultiprocess::Run(); + WinMultiprocessWithTempDir::Run(); } void StartAndUseBrokenHandler(CrashpadClient* client) { diff --git a/test/test.gyp b/test/test.gyp index f1adf9c2..94b198c7 100644 --- a/test/test.gyp +++ b/test/test.gyp @@ -61,6 +61,8 @@ 'win/win_child_process.h', 'win/win_multiprocess.cc', 'win/win_multiprocess.h', + 'win/win_multiprocess_with_temp_dir.cc', + 'win/win_multiprocess_with_temp_dir.h', ], 'direct_dependent_settings': { 'include_dirs': [ diff --git a/test/win/win_multiprocess_with_temp_dir.cc b/test/win/win_multiprocess_with_temp_dir.cc new file mode 100644 index 00000000..13e38a5b --- /dev/null +++ b/test/win/win_multiprocess_with_temp_dir.cc @@ -0,0 +1,188 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "test/win/win_multiprocess_with_temp_dir.h" + +#include + +#include "base/memory/ptr_util.h" +#include "test/errors.h" +#include "util/win/process_info.h" + +namespace crashpad { +namespace test { + +namespace { + +constexpr wchar_t kTempDirEnvName[] = L"CRASHPAD_TEST_TEMP_DIR"; + +// 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; + } + } +} + +} // namespace + +WinMultiprocessWithTempDir::WinMultiprocessWithTempDir() + : WinMultiprocess(), temp_dir_env_(kTempDirEnvName) {} + +void WinMultiprocessWithTempDir::WinMultiprocessParentBeforeChild() { + temp_dir_ = base::WrapUnique(new ScopedTempDir); + temp_dir_env_.SetValue(temp_dir_->path().value().c_str()); +} + +void WinMultiprocessWithTempDir::WinMultiprocessParentAfterChild(HANDLE child) { + WaitForAllChildProcessesOf(child); + temp_dir_.reset(); +} + +base::FilePath WinMultiprocessWithTempDir::GetTempDirPath() const { + return base::FilePath(temp_dir_env_.GetValue()); +} + +WinMultiprocessWithTempDir::ScopedEnvironmentVariable:: + ScopedEnvironmentVariable(const wchar_t* name) + : name_(name) { + original_value_ = GetValueImpl(&was_defined_); +} + +WinMultiprocessWithTempDir::ScopedEnvironmentVariable:: + ~ScopedEnvironmentVariable() { + if (was_defined_) + SetValue(original_value_.data()); + else + SetValue(nullptr); +} + +std::wstring WinMultiprocessWithTempDir::ScopedEnvironmentVariable::GetValue() + const { + bool dummy; + return GetValueImpl(&dummy); +} + +std::wstring +WinMultiprocessWithTempDir::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 WinMultiprocessWithTempDir::ScopedEnvironmentVariable::SetValue( + const wchar_t* new_value) const { + SetEnvironmentVariable(name_, new_value); +} + +} // namespace test +} // namespace crashpad diff --git a/test/win/win_multiprocess_with_temp_dir.h b/test/win/win_multiprocess_with_temp_dir.h new file mode 100644 index 00000000..61db8f76 --- /dev/null +++ b/test/win/win_multiprocess_with_temp_dir.h @@ -0,0 +1,80 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_TEST_WIN_WIN_MULTIPROCESS_WITH_TEMPDIR_H_ +#define CRASHPAD_TEST_WIN_WIN_MULTIPROCESS_WITH_TEMPDIR_H_ + +#include +#include + +#include + +#include "base/files/file_path.h" +#include "base/macros.h" +#include "test/scoped_temp_dir.h" +#include "test/win/win_multiprocess.h" + +namespace crashpad { +namespace test { + +//! \brief Manages a multiprocess test on Windows with a parent-created +//! temporary directory. +//! +//! This class creates a temp directory in the parent process for the use of +//! the subprocess and its children. To ensure a raceless rundown, it waits on +//! the child process and any processes directly created by the child before +//! deleting the temporary directory. +class WinMultiprocessWithTempDir : public WinMultiprocess { + public: + WinMultiprocessWithTempDir(); + + protected: + void WinMultiprocessParentBeforeChild() override; + void WinMultiprocessParentAfterChild(HANDLE child) override; + + //! \brief Returns the path of the temp directory. + base::FilePath GetTempDirPath() const; + + private: + 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); + }; + + std::unique_ptr temp_dir_; + ScopedEnvironmentVariable temp_dir_env_; + + DISALLOW_COPY_AND_ASSIGN(WinMultiprocessWithTempDir); +}; + +} // namespace test +} // namespace crashpad + +#endif // CRASHPAD_TEST_WIN_WIN_MULTIPROCESS_H_