diff options
| -rw-r--r-- | base/win/scoped_process_information.cc | 82 | ||||
| -rw-r--r-- | base/win/scoped_process_information.h | 57 | ||||
| -rw-r--r-- | base/win/scoped_process_information_unittest.cc | 165 | ||||
| -rw-r--r-- | sandbox/win/src/target_process.cc | 7 |
4 files changed, 144 insertions, 167 deletions
diff --git a/base/win/scoped_process_information.cc b/base/win/scoped_process_information.cc index 4adb8d4..cb7a30e 100644 --- a/base/win/scoped_process_information.cc +++ b/base/win/scoped_process_information.cc @@ -12,15 +12,6 @@ namespace win { namespace { -// Closes the provided handle if it is not NULL. -void CheckAndCloseHandle(HANDLE handle) { - if (!handle) - return; - if (::CloseHandle(handle)) - return; - CHECK(false); -} - // Duplicates source into target, returning true upon success. |target| is // guaranteed to be untouched in case of failure. Succeeds with no side-effects // if source is NULL. @@ -42,35 +33,38 @@ bool CheckAndDuplicateHandle(HANDLE source, HANDLE* target) { } // namespace ScopedProcessInformation::ScopedProcessInformation() - : process_information_() { + : process_id_(0), thread_id_(0) { } ScopedProcessInformation::~ScopedProcessInformation() { Close(); } -PROCESS_INFORMATION* ScopedProcessInformation::Receive() { +ScopedProcessInformation::Receiver ScopedProcessInformation::Receive() { DCHECK(!IsValid()) << "process_information_ must be NULL"; - return &process_information_; + return Receiver(this); } bool ScopedProcessInformation::IsValid() const { - return process_information_.hThread || process_information_.hProcess || - process_information_.dwProcessId || process_information_.dwThreadId; + return process_id_ || process_handle_.Get() || + thread_id_ || thread_handle_.Get(); } - void ScopedProcessInformation::Close() { - CheckAndCloseHandle(process_information_.hThread); - CheckAndCloseHandle(process_information_.hProcess); - Reset(); + process_handle_.Close(); + thread_handle_.Close(); + process_id_ = 0; + thread_id_ = 0; } -void ScopedProcessInformation::Swap(ScopedProcessInformation* other) { - DCHECK(other); - PROCESS_INFORMATION temp = other->process_information_; - other->process_information_ = process_information_; - process_information_ = temp; +void ScopedProcessInformation::Set(const PROCESS_INFORMATION& process_info) { + if (IsValid()) + Close(); + + process_handle_.Set(process_info.hProcess); + thread_handle_.Set(process_info.hThread); + process_id_ = process_info.dwProcessId; + thread_id_ = process_info.dwThreadId; } bool ScopedProcessInformation::DuplicateFrom( @@ -78,17 +72,12 @@ bool ScopedProcessInformation::DuplicateFrom( DCHECK(!IsValid()) << "target ScopedProcessInformation must be NULL"; DCHECK(other.IsValid()) << "source ScopedProcessInformation must be valid"; - ScopedHandle duplicate_process; - ScopedHandle duplicate_thread; - if (CheckAndDuplicateHandle(other.process_handle(), - duplicate_process.Receive()) && + process_handle_.Receive()) && CheckAndDuplicateHandle(other.thread_handle(), - duplicate_thread.Receive())) { - process_information_.dwProcessId = other.process_id(); - process_information_.dwThreadId = other.thread_id(); - process_information_.hProcess = duplicate_process.Take(); - process_information_.hThread = duplicate_thread.Take(); + thread_handle_.Receive())) { + process_id_ = other.process_id(); + thread_id_ = other.thread_id(); return true; } @@ -96,30 +85,25 @@ bool ScopedProcessInformation::DuplicateFrom( } PROCESS_INFORMATION ScopedProcessInformation::Take() { - PROCESS_INFORMATION process_information = process_information_; - Reset(); + PROCESS_INFORMATION process_information = {}; + process_information.hProcess = process_handle_.Take(); + process_information.hThread = thread_handle_.Take(); + process_information.dwProcessId = process_id(); + process_information.dwThreadId = thread_id(); + process_id_ = 0; + thread_id_ = 0; + return process_information; } HANDLE ScopedProcessInformation::TakeProcessHandle() { - HANDLE process = process_information_.hProcess; - process_information_.hProcess = NULL; - process_information_.dwProcessId = 0; - return process; + process_id_ = 0; + return process_handle_.Take(); } HANDLE ScopedProcessInformation::TakeThreadHandle() { - HANDLE thread = process_information_.hThread; - process_information_.hThread = NULL; - process_information_.dwThreadId = 0; - return thread; -} - -void ScopedProcessInformation::Reset() { - process_information_.hThread = NULL; - process_information_.hProcess = NULL; - process_information_.dwProcessId = 0; - process_information_.dwThreadId = 0; + thread_id_ = 0; + return thread_handle_.Take(); } } // namespace win diff --git a/base/win/scoped_process_information.h b/base/win/scoped_process_information.h index cfa7dc96..1f404c2 100644 --- a/base/win/scoped_process_information.h +++ b/base/win/scoped_process_information.h @@ -9,6 +9,7 @@ #include "base/basictypes.h" #include "base/base_export.h" +#include "base/win/scoped_handle.h" namespace base { namespace win { @@ -17,27 +18,41 @@ namespace win { // structures. Allows clients to take ownership of either handle independently. class BASE_EXPORT ScopedProcessInformation { public: - // Creates an instance holding a null PROCESS_INFORMATION. - ScopedProcessInformation(); + // Helper object to contain the effect of Receive() to the funtion that needs + // a pointer. + class Receiver { + public: + explicit Receiver(ScopedProcessInformation* owner) + : info_(), + owner_(owner) {} + ~Receiver() { owner_->Set(info_); } + + operator PROCESS_INFORMATION*() { return &info_; } + + private: + PROCESS_INFORMATION info_; + ScopedProcessInformation* owner_; + }; - // Closes the held thread and process handles, if any. + ScopedProcessInformation(); ~ScopedProcessInformation(); - // Returns a pointer that may be passed to API calls such as CreateProcess. + // Returns an object that may be passed to API calls such as CreateProcess. // DCHECKs that the object is not currently holding any handles. // HANDLEs stored in the returned PROCESS_INFORMATION will be owned by this // instance. - PROCESS_INFORMATION* Receive(); + // The intended use case is something like this: + // if (::CreateProcess(..., startup_info, scoped_proces_info.Receive())) + Receiver Receive(); // Returns true iff this instance is holding a thread and/or process handle. bool IsValid() const; - // Closes the held thread and process handles, if any, and resets the held - // PROCESS_INFORMATION to null. + // Closes the held thread and process handles, if any. void Close(); - // Swaps contents with the other ScopedProcessInformation. - void Swap(ScopedProcessInformation* other); + // Populates this instance with the provided |process_info|. + void Set(const PROCESS_INFORMATION& process_info); // Populates this instance with duplicate handles and the thread/process IDs // from |other|. Returns false in case of failure, in which case this instance @@ -45,44 +60,42 @@ class BASE_EXPORT ScopedProcessInformation { bool DuplicateFrom(const ScopedProcessInformation& other); // Transfers ownership of the held PROCESS_INFORMATION, if any, away from this - // instance. Resets the held PROCESS_INFORMATION to null. + // instance. PROCESS_INFORMATION Take(); // Transfers ownership of the held process handle, if any, away from this - // instance. The hProcess and dwProcessId members of the held - // PROCESS_INFORMATION will be reset. + // instance. Note that the related process_id will also be cleared. HANDLE TakeProcessHandle(); // Transfers ownership of the held thread handle, if any, away from this - // instance. The hThread and dwThreadId members of the held - // PROCESS_INFORMATION will be reset. + // instance. Note that the related thread_id will also be cleared. HANDLE TakeThreadHandle(); // Returns the held process handle, if any, while retaining ownership. HANDLE process_handle() const { - return process_information_.hProcess; + return process_handle_.Get(); } // Returns the held thread handle, if any, while retaining ownership. HANDLE thread_handle() const { - return process_information_.hThread; + return thread_handle_.Get(); } // Returns the held process id, if any. DWORD process_id() const { - return process_information_.dwProcessId; + return process_id_; } // Returns the held thread id, if any. DWORD thread_id() const { - return process_information_.dwThreadId; + return thread_id_; } private: - // Resets the held PROCESS_INFORMATION to null. - void Reset(); - - PROCESS_INFORMATION process_information_; + ScopedHandle process_handle_; + ScopedHandle thread_handle_; + DWORD process_id_; + DWORD thread_id_; DISALLOW_COPY_AND_ASSIGN(ScopedProcessInformation); }; diff --git a/base/win/scoped_process_information_unittest.cc b/base/win/scoped_process_information_unittest.cc index 906c156..75b69e37 100644 --- a/base/win/scoped_process_information_unittest.cc +++ b/base/win/scoped_process_information_unittest.cc @@ -12,6 +12,22 @@ #include "base/win/scoped_process_information.h" #include "testing/multiprocess_func_list.h" +namespace { + +const DWORD kProcessId = 4321; +const DWORD kThreadId = 1234; +const HANDLE kProcessHandle = reinterpret_cast<HANDLE>(7651); +const HANDLE kThreadHandle = reinterpret_cast<HANDLE>(1567); + +void MockCreateProcess(PROCESS_INFORMATION* process_info) { + process_info->dwProcessId = kProcessId; + process_info->dwThreadId = kThreadId; + process_info->hProcess = kProcessHandle; + process_info->hThread = kThreadHandle; +} + +} // namespace + class ScopedProcessInformationTest : public base::MultiProcessTest { protected: void DoCreateProcess(const std::string& main_id, @@ -39,76 +55,67 @@ void ScopedProcessInformationTest::DoCreateProcess( &startup_info, process_handle)); } +TEST_F(ScopedProcessInformationTest, InitiallyInvalid) { + base::win::ScopedProcessInformation process_info; + ASSERT_FALSE(process_info.IsValid()); +} + +TEST_F(ScopedProcessInformationTest, Receive) { + base::win::ScopedProcessInformation process_info; + MockCreateProcess(process_info.Receive()); + + EXPECT_TRUE(process_info.IsValid()); + EXPECT_EQ(kProcessId, process_info.process_id()); + EXPECT_EQ(kThreadId, process_info.thread_id()); + EXPECT_EQ(kProcessHandle, process_info.process_handle()); + EXPECT_EQ(kThreadHandle, process_info.thread_handle()); + PROCESS_INFORMATION to_discard = process_info.Take(); +} + TEST_F(ScopedProcessInformationTest, TakeProcess) { base::win::ScopedProcessInformation process_info; - DoCreateProcess("ReturnSeven", process_info.Receive()); - int exit_code = 0; - ASSERT_TRUE(base::WaitForExitCode(process_info.TakeProcessHandle(), - &exit_code)); - ASSERT_EQ(7, exit_code); - ASSERT_TRUE(process_info.IsValid()); - ASSERT_EQ(0u, process_info.process_id()); - ASSERT_TRUE(process_info.process_handle() == NULL); - ASSERT_NE(0u, process_info.thread_id()); - ASSERT_FALSE(process_info.thread_handle() == NULL); + MockCreateProcess(process_info.Receive()); + + HANDLE process = process_info.TakeProcessHandle(); + EXPECT_EQ(kProcessHandle, process); + EXPECT_EQ(NULL, process_info.process_handle()); + EXPECT_EQ(0, process_info.process_id()); + EXPECT_TRUE(process_info.IsValid()); + PROCESS_INFORMATION to_discard = process_info.Take(); } TEST_F(ScopedProcessInformationTest, TakeThread) { base::win::ScopedProcessInformation process_info; - DoCreateProcess("ReturnSeven", process_info.Receive()); - ASSERT_TRUE(::CloseHandle(process_info.TakeThreadHandle())); - ASSERT_TRUE(process_info.IsValid()); - ASSERT_NE(0u, process_info.process_id()); - ASSERT_FALSE(process_info.process_handle() == NULL); - ASSERT_EQ(0u, process_info.thread_id()); - ASSERT_TRUE(process_info.thread_handle() == NULL); + MockCreateProcess(process_info.Receive()); + + HANDLE thread = process_info.TakeThreadHandle(); + EXPECT_EQ(kThreadHandle, thread); + EXPECT_EQ(NULL, process_info.thread_handle()); + EXPECT_EQ(0, process_info.thread_id()); + EXPECT_TRUE(process_info.IsValid()); + PROCESS_INFORMATION to_discard = process_info.Take(); } TEST_F(ScopedProcessInformationTest, TakeBoth) { base::win::ScopedProcessInformation process_info; - DoCreateProcess("ReturnSeven", process_info.Receive()); - int exit_code = 0; - ASSERT_TRUE(base::WaitForExitCode(process_info.TakeProcessHandle(), - &exit_code)); - ASSERT_EQ(7, exit_code); - ASSERT_TRUE(::CloseHandle(process_info.TakeThreadHandle())); - ASSERT_FALSE(process_info.IsValid()); - ASSERT_EQ(0u, process_info.process_id()); - ASSERT_TRUE(process_info.process_handle() == NULL); - ASSERT_EQ(0u, process_info.thread_id()); - ASSERT_TRUE(process_info.thread_handle() == NULL); -} + MockCreateProcess(process_info.Receive()); -TEST_F(ScopedProcessInformationTest, TakeNothing) { - base::win::ScopedProcessInformation process_info; - DoCreateProcess("ReturnSeven", process_info.Receive()); - ASSERT_TRUE(process_info.IsValid()); - ASSERT_NE(0u, process_info.thread_id()); - ASSERT_FALSE(process_info.thread_handle() == NULL); - ASSERT_NE(0u, process_info.process_id()); - ASSERT_FALSE(process_info.process_handle() == NULL); + HANDLE process = process_info.TakeProcessHandle(); + HANDLE thread = process_info.TakeThreadHandle(); + EXPECT_FALSE(process_info.IsValid()); + PROCESS_INFORMATION to_discard = process_info.Take(); } TEST_F(ScopedProcessInformationTest, TakeWholeStruct) { base::win::ScopedProcessInformation process_info; - DoCreateProcess("ReturnSeven", process_info.Receive()); - base::win::ScopedProcessInformation other; - *other.Receive() = process_info.Take(); - - ASSERT_FALSE(process_info.IsValid()); - ASSERT_EQ(0u, process_info.process_id()); - ASSERT_TRUE(process_info.process_handle() == NULL); - ASSERT_EQ(0u, process_info.thread_id()); - ASSERT_TRUE(process_info.thread_handle() == NULL); - - // Validate that what was taken is good. - ASSERT_NE(0u, other.thread_id()); - ASSERT_NE(0u, other.process_id()); - int exit_code = 0; - ASSERT_TRUE(base::WaitForExitCode(other.TakeProcessHandle(), - &exit_code)); - ASSERT_EQ(7, exit_code); - ASSERT_TRUE(::CloseHandle(other.TakeThreadHandle())); + MockCreateProcess(process_info.Receive()); + + PROCESS_INFORMATION to_discard = process_info.Take(); + EXPECT_EQ(kProcessId, to_discard.dwProcessId); + EXPECT_EQ(kThreadId, to_discard.dwThreadId); + EXPECT_EQ(kProcessHandle, to_discard.hProcess); + EXPECT_EQ(kThreadHandle, to_discard.hThread); + EXPECT_FALSE(process_info.IsValid()); } TEST_F(ScopedProcessInformationTest, Duplicate) { @@ -138,44 +145,16 @@ TEST_F(ScopedProcessInformationTest, Duplicate) { ASSERT_TRUE(::CloseHandle(duplicate.TakeThreadHandle())); } -TEST_F(ScopedProcessInformationTest, Swap) { - base::win::ScopedProcessInformation seven_to_nine_info; - DoCreateProcess("ReturnSeven", seven_to_nine_info.Receive()); - base::win::ScopedProcessInformation nine_to_seven_info; - DoCreateProcess("ReturnNine", nine_to_seven_info.Receive()); - - HANDLE seven_process = seven_to_nine_info.process_handle(); - DWORD seven_process_id = seven_to_nine_info.process_id(); - HANDLE seven_thread = seven_to_nine_info.thread_handle(); - DWORD seven_thread_id = seven_to_nine_info.thread_id(); - HANDLE nine_process = nine_to_seven_info.process_handle(); - DWORD nine_process_id = nine_to_seven_info.process_id(); - HANDLE nine_thread = nine_to_seven_info.thread_handle(); - DWORD nine_thread_id = nine_to_seven_info.thread_id(); - - seven_to_nine_info.Swap(&nine_to_seven_info); - - ASSERT_EQ(seven_process, nine_to_seven_info.process_handle()); - ASSERT_EQ(seven_process_id, nine_to_seven_info.process_id()); - ASSERT_EQ(seven_thread, nine_to_seven_info.thread_handle()); - ASSERT_EQ(seven_thread_id, nine_to_seven_info.thread_id()); - ASSERT_EQ(nine_process, seven_to_nine_info.process_handle()); - ASSERT_EQ(nine_process_id, seven_to_nine_info.process_id()); - ASSERT_EQ(nine_thread, seven_to_nine_info.thread_handle()); - ASSERT_EQ(nine_thread_id, seven_to_nine_info.thread_id()); +TEST_F(ScopedProcessInformationTest, Set) { + PROCESS_INFORMATION base_process_info = {}; + MockCreateProcess(&base_process_info); - int exit_code = 0; - ASSERT_TRUE(base::WaitForExitCode(seven_to_nine_info.TakeProcessHandle(), - &exit_code)); - ASSERT_EQ(9, exit_code); - - ASSERT_TRUE(base::WaitForExitCode(nine_to_seven_info.TakeProcessHandle(), - &exit_code)); - ASSERT_EQ(7, exit_code); - -} - -TEST_F(ScopedProcessInformationTest, InitiallyInvalid) { base::win::ScopedProcessInformation process_info; - ASSERT_FALSE(process_info.IsValid()); + process_info.Set(base_process_info); + + EXPECT_EQ(kProcessId, process_info.process_id()); + EXPECT_EQ(kThreadId, process_info.thread_id()); + EXPECT_EQ(kProcessHandle, process_info.process_handle()); + EXPECT_EQ(kThreadHandle, process_info.thread_handle()); + base_process_info = process_info.Take(); } diff --git a/sandbox/win/src/target_process.cc b/sandbox/win/src/target_process.cc index 5de6ee0..2bc1fec 100644 --- a/sandbox/win/src/target_process.cc +++ b/sandbox/win/src/target_process.cc @@ -198,7 +198,7 @@ DWORD TargetProcess::Create(const wchar_t* exe_path, } base_address_ = GetBaseAddress(exe_path, entry_point); - sandbox_process_info_.Swap(&process_info); + sandbox_process_info_.Set(process_info.Take()); return win_result; } @@ -324,10 +324,11 @@ void TargetProcess::Terminate() { ::TerminateProcess(sandbox_process_info_.process_handle(), 0); } - TargetProcess* MakeTestTargetProcess(HANDLE process, HMODULE base_address) { TargetProcess* target = new TargetProcess(NULL, NULL, NULL, NULL); - target->sandbox_process_info_.Receive()->hProcess = process; + PROCESS_INFORMATION process_info = {}; + process_info.hProcess = process; + target->sandbox_process_info_.Set(process_info); target->base_address_ = base_address; return target; } |