From f357afc43e20b1e405dc06a59184f43feb71d9d0 Mon Sep 17 00:00:00 2001 From: Erik Wright Date: Wed, 13 May 2015 14:05:57 -0400 Subject: [PATCH] Move thread from test/ to util/thread/. BUG= R=scottmg@chromium.org Review URL: https://codereview.chromium.org/1134943003 --- test/test.gyp | 4 - util/file/file_io_test.cc | 2 +- {test => util/thread}/thread.cc | 8 +- {test => util/thread}/thread.h | 13 ++-- util/thread/thread_log_messages_test.cc | 2 +- {test => util/thread}/thread_posix.cc | 15 ++-- util/thread/thread_test.cc | 98 +++++++++++++++++++++++++ {test => util/thread}/thread_win.cc | 15 ++-- util/util.gyp | 4 + util/util_test.gyp | 1 + 10 files changed, 125 insertions(+), 37 deletions(-) rename {test => util/thread}/thread.cc (85%) rename {test => util/thread}/thread.h (83%) rename {test => util/thread}/thread_posix.cc (77%) create mode 100644 util/thread/thread_test.cc rename {test => util/thread}/thread_win.cc (76%) diff --git a/test/test.gyp b/test/test.gyp index 7c0175ca..0c6f92e9 100644 --- a/test/test.gyp +++ b/test/test.gyp @@ -51,10 +51,6 @@ 'scoped_temp_dir.h', 'scoped_temp_dir_posix.cc', 'scoped_temp_dir_win.cc', - 'thread.cc', - 'thread.h', - 'thread_posix.cc', - 'thread_win.cc', ], 'conditions': [ ['OS=="mac"', { diff --git a/util/file/file_io_test.cc b/util/file/file_io_test.cc index f91cde5b..5719dccb 100644 --- a/util/file/file_io_test.cc +++ b/util/file/file_io_test.cc @@ -20,7 +20,7 @@ #include "gtest/gtest.h" #include "test/errors.h" #include "test/scoped_temp_dir.h" -#include "test/thread.h" +#include "util/thread/thread.h" namespace crashpad { namespace test { diff --git a/test/thread.cc b/util/thread/thread.cc similarity index 85% rename from test/thread.cc rename to util/thread/thread.cc index baeeafa6..04782bb7 100644 --- a/test/thread.cc +++ b/util/thread/thread.cc @@ -12,19 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "test/thread.h" +#include "util/thread/thread.h" -#include "gtest/gtest.h" +#include "base/logging.h" namespace crashpad { -namespace test { Thread::Thread() : platform_thread_(0) { } Thread::~Thread() { - EXPECT_FALSE(platform_thread_); + DCHECK(!platform_thread_); } -} // namespace test } // namespace crashpad diff --git a/test/thread.h b/util/thread/thread.h similarity index 83% rename from test/thread.h rename to util/thread/thread.h index 46fe1e8e..b1801f83 100644 --- a/test/thread.h +++ b/util/thread/thread.h @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef CRASHPAD_TEST_THREAD_H_ -#define CRASHPAD_TEST_THREAD_H_ +#ifndef CRASHPAD_UTIL_THREAD_THREAD_H_ +#define CRASHPAD_UTIL_THREAD_THREAD_H_ #include "base/basictypes.h" #include "build/build_config.h" @@ -25,9 +25,8 @@ #endif // OS_POSIX namespace crashpad { -namespace test { -//! \brief Basic thread abstraction for testing. Users should derive from this +//! \brief Basic thread abstraction. Users should derive from this //! class and implement ThreadMain(). class Thread { public: @@ -43,8 +42,7 @@ class Thread { void Join(); private: - //! \brief The entry point to be overridden to implement the test-specific - //! functionality. + //! \brief The thread entry point to be implemented by the subclass. virtual void ThreadMain() = 0; static @@ -64,7 +62,6 @@ class Thread { DISALLOW_COPY_AND_ASSIGN(Thread); }; -} // namespace test } // namespace crashpad -#endif // CRASHPAD_TEST_THREAD_H_ +#endif // CRASHPAD_UTIL_THREAD_THREAD_H_ diff --git a/util/thread/thread_log_messages_test.cc b/util/thread/thread_log_messages_test.cc index 96e2e91a..e2590826 100644 --- a/util/thread/thread_log_messages_test.cc +++ b/util/thread/thread_log_messages_test.cc @@ -19,7 +19,7 @@ #include "base/logging.h" #include "base/strings/stringprintf.h" #include "gtest/gtest.h" -#include "test/thread.h" +#include "util/thread/thread.h" namespace crashpad { namespace test { diff --git a/test/thread_posix.cc b/util/thread/thread_posix.cc similarity index 77% rename from test/thread_posix.cc rename to util/thread/thread_posix.cc index 9334d89a..7142c786 100644 --- a/test/thread_posix.cc +++ b/util/thread/thread_posix.cc @@ -12,24 +12,22 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "test/thread.h" +#include "util/thread/thread.h" -#include "gtest/gtest.h" -#include "test/errors.h" +#include "base/logging.h" namespace crashpad { -namespace test { void Thread::Start() { - ASSERT_FALSE(platform_thread_); + DCHECK(!platform_thread_); int rv = pthread_create(&platform_thread_, nullptr, ThreadEntryThunk, this); - ASSERT_EQ(0, rv) << ErrnoMessage(rv, "pthread_create"); + PCHECK(0 == rv); } void Thread::Join() { - ASSERT_TRUE(platform_thread_); + DCHECK(platform_thread_); int rv = pthread_join(platform_thread_, nullptr); - EXPECT_EQ(0, rv) << ErrnoMessage(rv, "pthread_join"); + PCHECK(0 == rv); platform_thread_ = 0; } @@ -40,5 +38,4 @@ void* Thread::ThreadEntryThunk(void* argument) { return nullptr; } -} // namespace test } // namespace crashpad diff --git a/util/thread/thread_test.cc b/util/thread/thread_test.cc new file mode 100644 index 00000000..7f7cf615 --- /dev/null +++ b/util/thread/thread_test.cc @@ -0,0 +1,98 @@ +// Copyright 2015 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 "util/thread/thread.h" + +#include "base/basictypes.h" +#include "gtest/gtest.h" +#include "util/synchronization/semaphore.h" + +namespace crashpad { +namespace test { +namespace { + +class NoopThread : public Thread { + public: + NoopThread() {} + ~NoopThread() override {} + + private: + void ThreadMain() override {} + + DISALLOW_COPY_AND_ASSIGN(NoopThread); +}; + +class WaitThread : public Thread { + public: + explicit WaitThread(Semaphore* semaphore) : semaphore_(semaphore) {} + ~WaitThread() override {} + + private: + void ThreadMain() override { semaphore_->Wait(); } + + Semaphore* semaphore_; + + DISALLOW_COPY_AND_ASSIGN(WaitThread); +}; + +class JoinAndSignalThread : public Thread { + public: + JoinAndSignalThread(Thread* thread, Semaphore* semaphore) + : thread_(thread), semaphore_(semaphore) {} + ~JoinAndSignalThread() override {} + + private: + void ThreadMain() override { + thread_->Join(); + semaphore_->Signal(); + } + + Thread* thread_; + Semaphore* semaphore_; + + DISALLOW_COPY_AND_ASSIGN(JoinAndSignalThread); +}; + +TEST(ThreadTest, NoStart) { + NoopThread thread; +} + +TEST(ThreadTest, Start) { + NoopThread thread; + thread.Start(); + thread.Join(); +} + +TEST(ThreadTest, JoinBlocks) { + Semaphore unblock_wait_thread_semaphore(0); + Semaphore join_completed_semaphore(0); + WaitThread wait_thread(&unblock_wait_thread_semaphore); + wait_thread.Start(); + JoinAndSignalThread join_and_signal_thread(&wait_thread, + &join_completed_semaphore); + join_and_signal_thread.Start(); + // join_completed_semaphore will be signaled when wait_thread.Join() returns + // (in JoinAndSignalThread::ThreadMain). Since wait_thread is blocking on + // unblock_wait_thread_semaphore, we don't expect the Join to return yet. We + // wait up to 100ms to give a broken implementation of Thread::Join a chance + // to return. + ASSERT_FALSE(join_completed_semaphore.TimedWait(.1)); + unblock_wait_thread_semaphore.Signal(); + join_completed_semaphore.Wait(); + join_and_signal_thread.Join(); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/test/thread_win.cc b/util/thread/thread_win.cc similarity index 76% rename from test/thread_win.cc rename to util/thread/thread_win.cc index 05c67ea8..c4ac1eb7 100644 --- a/test/thread_win.cc +++ b/util/thread/thread_win.cc @@ -12,25 +12,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "test/thread.h" +#include "util/thread/thread.h" -#include "gtest/gtest.h" -#include "test/errors.h" +#include "base/logging.h" namespace crashpad { -namespace test { void Thread::Start() { - ASSERT_FALSE(platform_thread_); + DCHECK(!platform_thread_); platform_thread_ = CreateThread(nullptr, 0, ThreadEntryThunk, this, 0, nullptr); - ASSERT_TRUE(platform_thread_) << ErrorMessage("CreateThread"); + PCHECK(platform_thread_); } void Thread::Join() { - ASSERT_TRUE(platform_thread_); + DCHECK(platform_thread_); DWORD result = WaitForSingleObject(platform_thread_, INFINITE); - EXPECT_EQ(WAIT_OBJECT_0, result) << ErrorMessage("WaitForSingleObject"); + PCHECK(WAIT_OBJECT_0 == result); platform_thread_ = 0; } @@ -41,5 +39,4 @@ DWORD WINAPI Thread::ThreadEntryThunk(void* argument) { return 0; } -} // namespace test } // namespace crashpad diff --git a/util/util.gyp b/util/util.gyp index e1bb1299..3bfa2585 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -139,8 +139,12 @@ 'synchronization/semaphore_posix.cc', 'synchronization/semaphore_win.cc', 'synchronization/semaphore.h', + 'thread/thread.cc', + 'thread/thread.h', 'thread/thread_log_messages.cc', 'thread/thread_log_messages.h', + 'thread/thread_posix.cc', + 'thread/thread_win.cc', 'win/address_types.h', 'win/checked_win_address_range.h', 'win/process_info.cc', diff --git a/util/util_test.gyp b/util/util_test.gyp index 82a1c3b3..e2af6e0c 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -77,6 +77,7 @@ 'string/split_string_test.cc', 'synchronization/semaphore_test.cc', 'thread/thread_log_messages_test.cc', + 'thread/thread_test.cc', 'win/process_info_test.cc', 'win/time_test.cc', ],