diff options
| -rw-r--r-- | base/base.xcodeproj/project.pbxproj | 12 | ||||
| -rw-r--r-- | base/base_lib.scons | 1 | ||||
| -rw-r--r-- | base/base_unittests.scons | 1 | ||||
| -rw-r--r-- | base/build/base.vcproj | 8 | ||||
| -rw-r--r-- | base/build/base_unittests.vcproj | 4 | ||||
| -rw-r--r-- | base/condition_variable_unittest.cc | 15 | ||||
| -rw-r--r-- | base/revocable_store.h | 2 | ||||
| -rw-r--r-- | base/thread_collision_warner.cc | 53 | ||||
| -rw-r--r-- | base/thread_collision_warner.h | 243 | ||||
| -rw-r--r-- | base/thread_collision_warner_unittest.cc | 360 | ||||
| -rw-r--r-- | chrome/browser/printing/print_view_manager.cc | 3 | ||||
| -rw-r--r-- | chrome/browser/printing/print_view_manager.h | 1 |
12 files changed, 702 insertions, 1 deletions
diff --git a/base/base.xcodeproj/project.pbxproj b/base/base.xcodeproj/project.pbxproj index 28013bd..152b78c 100644 --- a/base/base.xcodeproj/project.pbxproj +++ b/base/base.xcodeproj/project.pbxproj @@ -35,6 +35,9 @@ /* End PBXAggregateTarget section */ /* Begin PBXBuildFile section */ + 141593B80EA63EBE00E32418 /* thread_collision_warner_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 146C6A6E0EA63D970029E7B6 /* thread_collision_warner_unittest.cc */; }; + 146C6A6B0EA63D4F0029E7B6 /* thread_collision_warner.cc in Sources */ = {isa = PBXBuildFile; fileRef = 146C6A620EA63CAE0029E7B6 /* thread_collision_warner.cc */; }; + 146C6A6C0EA63D4F0029E7B6 /* thread_collision_warner.h in Sources */ = {isa = PBXBuildFile; fileRef = 146C6A610EA63C9F0029E7B6 /* thread_collision_warner.h */; }; 4D11B59A0E91730200EF7617 /* rand_util.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D11B5940E9172F800EF7617 /* rand_util.cc */; }; 4D11B59B0E91730200EF7617 /* rand_util_posix.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D11B5960E9172F800EF7617 /* rand_util_posix.cc */; }; 4D11B59C0E91730500EF7617 /* rand_util_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4D11B5970E9172F800EF7617 /* rand_util_unittest.cc */; }; @@ -373,6 +376,9 @@ /* End PBXContainerItemProxy section */ /* Begin PBXFileReference section */ + 146C6A610EA63C9F0029E7B6 /* thread_collision_warner.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = thread_collision_warner.h; sourceTree = "<group>"; }; + 146C6A620EA63CAE0029E7B6 /* thread_collision_warner.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = thread_collision_warner.cc; sourceTree = "<group>"; }; + 146C6A6E0EA63D970029E7B6 /* thread_collision_warner_unittest.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = thread_collision_warner_unittest.cc; sourceTree = "<group>"; }; 4D11B5940E9172F800EF7617 /* rand_util.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = rand_util.cc; sourceTree = "<group>"; }; 4D11B5950E9172F800EF7617 /* rand_util.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = rand_util.h; sourceTree = "<group>"; }; 4D11B5960E9172F800EF7617 /* rand_util_posix.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = rand_util_posix.cc; sourceTree = "<group>"; }; @@ -1011,6 +1017,9 @@ 84B581950EDC6ECC00A6B5B3 /* test_file_util_mac.cc */, 93E703230E5D64F00046259B /* thread.cc */, 825403800D92D2CF0006B936 /* thread.h */, + 146C6A6E0EA63D970029E7B6 /* thread_collision_warner_unittest.cc */, + 146C6A620EA63CAE0029E7B6 /* thread_collision_warner.cc */, + 146C6A610EA63C9F0029E7B6 /* thread_collision_warner.h */, 7BAE38A80E6EFD9900C3F750 /* thread_local.h */, 7BAE38A90E6EFD9900C3F750 /* thread_local_posix.cc */, 825403820D92D2CF0006B936 /* thread_local_storage.h */, @@ -1399,6 +1408,8 @@ E4CE9D7A0E8C1FD400D5378C /* system_monitor.cc in Sources */, 84B581DC0EDC715A00A6B5B3 /* test_file_util_mac.cc in Sources */, 93E703240E5D64F00046259B /* thread.cc in Sources */, + 146C6A6B0EA63D4F0029E7B6 /* thread_collision_warner.cc in Sources */, + 146C6A6C0EA63D4F0029E7B6 /* thread_collision_warner.h in Sources */, 7BAE38AC0E6EFDBA00C3F750 /* thread_local_posix.cc in Sources */, 829E36730DC0FBAD00819EBF /* thread_local_storage_posix.cc in Sources */, 824654910DC25A8C007C2BAA /* time.cc in Sources */, @@ -1471,6 +1482,7 @@ 7B78D3A10E54FE0100609465 /* string_util_unittest.cc in Sources */, 7B6AF6350E80211E00F9F9CF /* sys_info_unittest.cc in Sources */, B5D544AB0EAFB7E000272A1C /* sys_string_conversions_unittest.cc in Sources */, + 141593B80EA63EBE00E32418 /* thread_collision_warner_unittest.cc in Sources */, BA0F69870E79D7980079A8A1 /* thread_local_storage_unittest.cc in Sources */, 7BAE38AF0E6EFDC300C3F750 /* thread_local_unittest.cc in Sources */, 93E7031B0E5D64390046259B /* thread_unittest.cc in Sources */, diff --git a/base/base_lib.scons b/base/base_lib.scons index e2bb18d..6ba8d6e 100644 --- a/base/base_lib.scons +++ b/base/base_lib.scons @@ -69,6 +69,7 @@ input_files = [ 'third_party/nspr/prtime.cc', 'third_party/nss/sha512.cc', 'thread.cc', + 'thread_collision_warner.cc', 'time.cc', 'time_format.cc', 'timer.cc', diff --git a/base/base_unittests.scons b/base/base_unittests.scons index 433fdcce..7aa039a 100644 --- a/base/base_unittests.scons +++ b/base/base_unittests.scons @@ -80,6 +80,7 @@ input_files = [ 'string_tokenizer_unittest.cc', 'string_util_unittest.cc', 'sys_info_unittest.cc', + 'thread_collision_warner_unittest.cc', 'thread_local_storage_unittest.cc', 'thread_local_unittest.cc', 'thread_unittest.cc', diff --git a/base/build/base.vcproj b/base/build/base.vcproj index 6202baa..10c7dad 100644 --- a/base/build/base.vcproj +++ b/base/build/base.vcproj @@ -850,6 +850,14 @@ > </File> <File + RelativePath="..\thread_collision_warner.cc" + > + </File> + <File + RelativePath="..\thread_collision_warner.h" + > + </File> + <File RelativePath="..\thread_local.h" > </File> diff --git a/base/build/base_unittests.vcproj b/base/build/base_unittests.vcproj index e822992..8263e074 100644 --- a/base/build/base_unittests.vcproj +++ b/base/build/base_unittests.vcproj @@ -336,6 +336,10 @@ > </File> <File + RelativePath="..\thread_collision_warner_unittest.cc" + > + </File> + <File RelativePath="..\thread_local_storage_unittest.cc" > </File> diff --git a/base/condition_variable_unittest.cc b/base/condition_variable_unittest.cc index c996f2c..8ca4bc1 100644 --- a/base/condition_variable_unittest.cc +++ b/base/condition_variable_unittest.cc @@ -13,6 +13,7 @@ #include "base/platform_thread.h" #include "base/scoped_ptr.h" #include "base/spin_wait.h" +#include "base/thread_collision_warner.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" @@ -79,6 +80,9 @@ class WorkQueue : public PlatformThread::Delegate { int shutdown_task_count() const; void thread_shutting_down(); + + //---------------------------------------------------------------------------- + // Worker threads can call them but not needed to acquire a lock Lock* lock(); ConditionVariable* work_is_available(); @@ -120,6 +124,8 @@ class WorkQueue : public PlatformThread::Delegate { TimeDelta worker_delay_; // Time each task takes to complete. bool allow_help_requests_; // Workers can signal more workers. bool shutdown_; // Set when threads need to terminate. + + DFAKE_MUTEX(locked_methods_); }; //------------------------------------------------------------------------------ @@ -477,15 +483,18 @@ WorkQueue::~WorkQueue() { } int WorkQueue::GetThreadId() { + DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_); DCHECK(!EveryIdWasAllocated()); return thread_started_counter_++; // Give out Unique IDs. } bool WorkQueue::EveryIdWasAllocated() const { + DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_); return thread_count_ == thread_started_counter_; } TimeDelta WorkQueue::GetAnAssignment(int thread_id) { + DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_); DCHECK_LT(0, task_count_); assignment_history_[thread_id]++; if (0 == --task_count_) { @@ -495,26 +504,32 @@ TimeDelta WorkQueue::GetAnAssignment(int thread_id) { } void WorkQueue::WorkIsCompleted(int thread_id) { + DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_); completion_history_[thread_id]++; } int WorkQueue::task_count() const { + DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_); return task_count_; } bool WorkQueue::allow_help_requests() const { + DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_); return allow_help_requests_; } bool WorkQueue::shutdown() const { + DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_); return shutdown_; } int WorkQueue::shutdown_task_count() const { + DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_); return shutdown_task_count_; } void WorkQueue::thread_shutting_down() { + DFAKE_SCOPED_RECURSIVE_LOCK(locked_methods_); shutdown_task_count_++; } diff --git a/base/revocable_store.h b/base/revocable_store.h index 65fb05e..2ee9705 100644 --- a/base/revocable_store.h +++ b/base/revocable_store.h @@ -17,7 +17,7 @@ class RevocableStore { // store wishes to revoke its items, it sets |store_| to null. Items are // permitted to release their reference to the |StoreRef| when they no longer // require the store. - class StoreRef : public base::RefCounted<StoreRef> { + class StoreRef : public base::RefCountedThreadSafe<StoreRef> { public: StoreRef(RevocableStore* store) : store_(store) { } diff --git a/base/thread_collision_warner.cc b/base/thread_collision_warner.cc new file mode 100644 index 0000000..1c51a3e --- /dev/null +++ b/base/thread_collision_warner.cc @@ -0,0 +1,53 @@ +// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/thread_collision_warner.h" + +#include "base/atomicops.h" +#include "base/logging.h" + +namespace base { + +void DCheckAsserter::warn() { + NOTREACHED() << "Thread Collision"; +} + +void ThreadCollisionWarner::EnterSelf() { + // If the active thread is 0 then I'll write the current thread ID + // if two or more threads arrive here only one will succeed to + // write on valid_thread_id_ the current thread ID. + const int current_thread_id = PlatformThread::CurrentId(); + + int previous_value = subtle::NoBarrier_CompareAndSwap(&valid_thread_id_, + 0, + current_thread_id); + if (previous_value != 0 && previous_value != current_thread_id) { + // gotcha! a thread is trying to use the same class and that is + // not current thread. + asserter_->warn(); + } + + subtle::NoBarrier_AtomicIncrement(&counter_, 1); +} + +void ThreadCollisionWarner::Enter() { + const int current_thread_id = PlatformThread::CurrentId(); + + if (subtle::NoBarrier_CompareAndSwap(&valid_thread_id_, + 0, + current_thread_id) != 0) { + // gotcha! another thread is trying to use the same class. + asserter_->warn(); + } + + subtle::NoBarrier_AtomicIncrement(&counter_, 1); +} + +void ThreadCollisionWarner::Leave() { + if (subtle::Barrier_AtomicIncrement(&counter_, -1) == 0) { + subtle::NoBarrier_Store(&valid_thread_id_, 0); + } +} + +} // namespace base diff --git a/base/thread_collision_warner.h b/base/thread_collision_warner.h new file mode 100644 index 0000000..fa8b7ee --- /dev/null +++ b/base/thread_collision_warner.h @@ -0,0 +1,243 @@ +// Copyright (c) 2008 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef THREAD_COLLISION_WARNER_H_ +#define THREAD_COLLISION_WARNER_H_ + +#include <memory> + +#include "base/atomicops.h" +#include "base/platform_thread.h" + +// A helper class alongside macros to be used to verify assumptions about thread +// safety of a class. +// +// Example: Queue implementation non thread-safe but still usable if clients +// are synchronized somehow. +// +// In this case the macro DFAKE_SCOPED_LOCK has to be +// used, it checks that if a thread is inside the push/pop then +// noone else is still inside the pop/push +// +// class NonThreadSafeQueue { +// public: +// ... +// void push(int) { DFAKE_SCOPED_LOCK(push_pop_); ... } +// int pop() { DFAKE_SCOPED_LOCK(push_pop_); ... } +// ... +// private: +// DFAKE_MUTEX(push_pop_); +// }; +// +// +// Example: Queue implementation non thread-safe but still usable if clients +// are synchronized somehow, it calls a method to "protect" from +// a "protected" method +// +// In this case the macro DFAKE_SCOPED_RECURSIVE_LOCK +// has to be used, it checks that if a thread is inside the push/pop +// then noone else is still inside the pop/push +// +// class NonThreadSafeQueue { +// public: +// void push(int) { +// DFAKE_SCOPED_LOCK(push_pop_); +// ... +// } +// int pop() { +// DFAKE_SCOPED_RECURSIVE_LOCK(push_pop_); +// bar(); +// ... +// } +// void bar() { DFAKE_SCOPED_RECURSIVE_LOCK(push_pop_); ... } +// ... +// private: +// DFAKE_MUTEX(push_pop_); +// }; +// +// +// Example: Queue implementation not usable even if clients are synchronized, +// so only one thread in the class life cycle can use the two members +// push/pop. +// +// In this case the macro DFAKE_SCOPED_LOCK_THREAD_LOCKED pins the +// specified +// critical section the first time a thread enters push or pop, from +// that time on only that thread is allowed to execute push or pop. +// +// class NonThreadSafeQueue { +// public: +// ... +// void push(int) { DFAKE_SCOPED_LOCK_THREAD_LOCKED(push_pop_); ... } +// int pop() { DFAKE_SCOPED_LOCK_THREAD_LOCKED(push_pop_); ... } +// ... +// private: +// DFAKE_MUTEX(push_pop_); +// }; +// +// +// Example: Class that has to be contructed/destroyed on same thread, it has +// a "shareable" method (with external syncronization) and a not +// shareable method (even with external synchronization). +// +// In this case 3 Critical sections have to be defined +// +// class ExoticClass { +// public: +// ExoticClass() { DFAKE_SCOPED_LOCK_THREAD_LOCKED(ctor_dtor_); ... } +// ~ExoticClass() { DFAKE_SCOPED_LOCK_THREAD_LOCKED(ctor_dtor_); ... } +// +// void Shareable() { DFAKE_SCOPED_LOCK(shareable_section_); ... } +// void NotShareable() { DFAKE_SCOPED_LOCK_THREAD_LOCKED(ctor_dtor_); ... } +// ... +// private: +// DFAKE_MUTEX(ctor_dtor_); +// DFAKE_MUTEX(shareable_section_); +// }; + + +#if !defined(NDEBUG) || defined(UNIT_TEST) + +// Defines a class member that acts like a mutex. It is used only as a +// verification tool. +#define DFAKE_MUTEX(obj) \ + mutable base::ThreadCollisionWarner obj +// Asserts the call is never called simultaneously in two threads. Used at +// member function scope. +#define DFAKE_SCOPED_LOCK(obj) \ + base::ThreadCollisionWarner::ScopedCheck s_check_##obj(&obj) +// Asserts the call is never called simultaneously in two threads. Used at +// member function scope. Same as DFAKE_SCOPED_LOCK but allows recursive locks. +#define DFAKE_SCOPED_RECURSIVE_LOCK(obj) \ + base::ThreadCollisionWarner::ScopedRecursiveCheck sr_check_##obj(&obj) +// Asserts the code is always executed in the same thread. +#define DFAKE_SCOPED_LOCK_THREAD_LOCKED(obj) \ + base::ThreadCollisionWarner::Check check_##obj(&obj) + +#else + +#define DFAKE_MUTEX(obj) +#define DFAKE_SCOPED_LOCK(obj) ((void)0) +#define DFAKE_SCOPED_RECURSIVE_LOCK(obj) ((void)0) +#define DFAKE_SCOPED_LOCK_THREAD_LOCKED(obj) ((void)0) + +#endif + +namespace base { + +// The class ThreadCollisionWarner uses an Asserter to notify the collision +// AsserterBase is the interfaces and DCheckAsserter is the default asserter +// used. During the unit tests is used another class that doesn't "DCHECK" +// in case of collision (check thread_collision_warner_unittests.cc) +struct AsserterBase { + virtual ~AsserterBase() {} + virtual void warn() = 0; +}; + +struct DCheckAsserter : public AsserterBase { + virtual ~DCheckAsserter() {} + virtual void warn(); +}; + +class ThreadCollisionWarner { + public: + // The parameter asserter is there only for test purpose + ThreadCollisionWarner(AsserterBase* asserter = new DCheckAsserter()) + : valid_thread_id_(0), + counter_(0), + asserter_(asserter) {} + + ~ThreadCollisionWarner() { + delete asserter_; + } + + // This class is meant to be used through the macro + // DFAKE_SCOPED_LOCK_THREAD_LOCKED + // it doesn't leave the critical section, as opposed to ScopedCheck, + // because the critical section being pinned is allowed to be used only + // from one thread + class Check { + public: + explicit Check(ThreadCollisionWarner* warner) + : warner_(warner) { + warner_->EnterSelf(); + } + + ~Check() {} + + private: + ThreadCollisionWarner* warner_; + + DISALLOW_COPY_AND_ASSIGN(Check); + }; + + // This class is meant to be used through the macro + // DFAKE_SCOPED_LOCK + class ScopedCheck { + public: + explicit ScopedCheck(ThreadCollisionWarner* warner) + : warner_(warner) { + warner_->Enter(); + } + + ~ScopedCheck() { + warner_->Leave(); + } + + private: + ThreadCollisionWarner* warner_; + + DISALLOW_COPY_AND_ASSIGN(ScopedCheck); + }; + + // This class is meant to be used through the macro + // DFAKE_SCOPED_RECURSIVE_LOCK + class ScopedRecursiveCheck { + public: + explicit ScopedRecursiveCheck(ThreadCollisionWarner* warner) + : warner_(warner) { + warner_->EnterSelf(); + } + + ~ScopedRecursiveCheck() { + warner_->Leave(); + } + + private: + ThreadCollisionWarner* warner_; + + DISALLOW_COPY_AND_ASSIGN(ScopedRecursiveCheck); + }; + + private: + // This method stores the current thread identifier and does a DCHECK + // if a another thread has already done it, it is safe if same thread + // calls this multiple time (recursion allowed). + void EnterSelf(); + + // Same as EnterSelf but recursion is not allowed. + void Enter(); + + // Removes the thread_id stored in order to allow other threads to + // call EnterSelf or Enter. + void Leave(); + + // This stores the thread id that is inside the critical section, if the + // value is 0 then no thread is inside. + volatile int valid_thread_id_; + + // Counter to trace how many time a critical section was "pinned" + // (when allowed) in order to unpin it when counter_ reaches 0. + volatile subtle::Atomic32 counter_; + + // Here only for class unit tests purpose, during the test I need to not + // DCHECK but notify the collision with something else. + AsserterBase* asserter_; + + DISALLOW_COPY_AND_ASSIGN(ThreadCollisionWarner); +}; + +} // namespace base + +#endif // THREAD_COLLISION_WARNER_H_ diff --git a/base/thread_collision_warner_unittest.cc b/base/thread_collision_warner_unittest.cc new file mode 100644 index 0000000..7994d13 --- /dev/null +++ b/base/thread_collision_warner_unittest.cc @@ -0,0 +1,360 @@ +// Copyright (c) 2008 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/compiler_specific.h" +#include "base/lock.h" +#include "base/platform_thread.h" +#include "base/simple_thread.h" +#include "base/thread_collision_warner.h" +#include "testing/gtest/include/gtest/gtest.h" + +// '' : local class member function does not have a body +MSVC_PUSH_DISABLE_WARNING(4822) + +namespace { + +// This is the asserter used with ThreadCollisionWarner instead of the default +// DCheckAsserter. The method fail_state is used to know if a collision took +// place. +class AssertReporter : public base::AsserterBase { + public: + AssertReporter() + : failed_(false) {} + + virtual void warn() { + failed_ = true; + } + + virtual ~AssertReporter() {} + + bool fail_state() const { return failed_; } + void reset() { failed_ = false; } + + private: + bool failed_; +}; + +} // namespace + +TEST(ThreadCollisionTest, BookCriticalSection) { + AssertReporter* local_reporter = new AssertReporter(); + + base::ThreadCollisionWarner warner(local_reporter); + EXPECT_FALSE(local_reporter->fail_state()); + + { // Pin section. + DFAKE_SCOPED_LOCK_THREAD_LOCKED(warner); + EXPECT_FALSE(local_reporter->fail_state()); + { // Pin section. + DFAKE_SCOPED_LOCK_THREAD_LOCKED(warner); + EXPECT_FALSE(local_reporter->fail_state()); + } + } +} + +TEST(ThreadCollisionTest, ScopedRecursiveBookCriticalSection) { + AssertReporter* local_reporter = new AssertReporter(); + + base::ThreadCollisionWarner warner(local_reporter); + EXPECT_FALSE(local_reporter->fail_state()); + + { // Pin section. + DFAKE_SCOPED_RECURSIVE_LOCK(warner); + EXPECT_FALSE(local_reporter->fail_state()); + { // Pin section again (allowed by DFAKE_SCOPED_RECURSIVE_LOCK) + DFAKE_SCOPED_RECURSIVE_LOCK(warner); + EXPECT_FALSE(local_reporter->fail_state()); + } // Unpin section. + } // Unpin section. + + // Check that section is not pinned + { // Pin section. + DFAKE_SCOPED_LOCK(warner); + EXPECT_FALSE(local_reporter->fail_state()); + } // Unpin section. +} + +TEST(ThreadCollisionTest, ScopedBookCriticalSection) { + AssertReporter* local_reporter = new AssertReporter(); + + base::ThreadCollisionWarner warner(local_reporter); + EXPECT_FALSE(local_reporter->fail_state()); + + { // Pin section. + DFAKE_SCOPED_LOCK(warner); + EXPECT_FALSE(local_reporter->fail_state()); + } // Unpin section. + + { // Pin section. + DFAKE_SCOPED_LOCK(warner); + EXPECT_FALSE(local_reporter->fail_state()); + { // Pin section again (not allowed by DFAKE_SCOPED_LOCK) + DFAKE_SCOPED_LOCK(warner); + EXPECT_TRUE(local_reporter->fail_state()); + // Reset the status of warner for further tests. + local_reporter->reset(); + } // Unpin section. + } // Unpin section. + + { // Pin section. + DFAKE_SCOPED_LOCK(warner); + EXPECT_FALSE(local_reporter->fail_state()); + } // Unpin section. +} + +TEST(ThreadCollisionTest, MTBookCriticalSectionTest) { + class NonThreadSafeQueue { + public: + explicit NonThreadSafeQueue(base::AsserterBase* asserter) + : push_pop_(asserter) { } + + void push(int value) { + DFAKE_SCOPED_LOCK_THREAD_LOCKED(push_pop_); + } + + int pop() { + DFAKE_SCOPED_LOCK_THREAD_LOCKED(push_pop_); + return 0; + } + + private: + DFAKE_MUTEX(push_pop_); + + DISALLOW_COPY_AND_ASSIGN(NonThreadSafeQueue); + }; + + class QueueUser : public base::DelegateSimpleThread::Delegate { + public: + explicit QueueUser(NonThreadSafeQueue& queue) + : queue_(queue) {} + + virtual void Run() { + queue_.push(0); + queue_.pop(); + } + + private: + NonThreadSafeQueue& queue_; + }; + + AssertReporter* local_reporter = new AssertReporter(); + + NonThreadSafeQueue queue(local_reporter); + + QueueUser queue_user_a(queue); + QueueUser queue_user_b(queue); + + base::DelegateSimpleThread thread_a(&queue_user_a, "queue_user_thread_a"); + base::DelegateSimpleThread thread_b(&queue_user_b, "queue_user_thread_b"); + + thread_a.Start(); + thread_b.Start(); + + thread_a.Join(); + thread_b.Join(); + + EXPECT_TRUE(local_reporter->fail_state()); +} + +TEST(ThreadCollisionTest, MTScopedBookCriticalSectionTest) { + // Queue with a 5 seconds push execution time, hopefuly the two used threads + // in the test will enter the push at same time. + class NonThreadSafeQueue { + public: + explicit NonThreadSafeQueue(base::AsserterBase* asserter) + : push_pop_(asserter) { } + + void push(int value) { + DFAKE_SCOPED_LOCK(push_pop_); + PlatformThread::Sleep(5000); + } + + int pop() { + DFAKE_SCOPED_LOCK(push_pop_); + return 0; + } + + private: + DFAKE_MUTEX(push_pop_); + + DISALLOW_COPY_AND_ASSIGN(NonThreadSafeQueue); + }; + + class QueueUser : public base::DelegateSimpleThread::Delegate { + public: + explicit QueueUser(NonThreadSafeQueue& queue) + : queue_(queue) {} + + virtual void Run() { + queue_.push(0); + queue_.pop(); + } + + private: + NonThreadSafeQueue& queue_; + }; + + AssertReporter* local_reporter = new AssertReporter(); + + NonThreadSafeQueue queue(local_reporter); + + QueueUser queue_user_a(queue); + QueueUser queue_user_b(queue); + + base::DelegateSimpleThread thread_a(&queue_user_a, "queue_user_thread_a"); + base::DelegateSimpleThread thread_b(&queue_user_b, "queue_user_thread_b"); + + thread_a.Start(); + thread_b.Start(); + + thread_a.Join(); + thread_b.Join(); + + EXPECT_TRUE(local_reporter->fail_state()); +} + +TEST(ThreadCollisionTest, MTSynchedScopedBookCriticalSectionTest) { + // Queue with a 5 seconds push execution time, hopefuly the two used threads + // in the test will enter the push at same time. + class NonThreadSafeQueue { + public: + explicit NonThreadSafeQueue(base::AsserterBase* asserter) + : push_pop_(asserter) { } + + void push(int value) { + DFAKE_SCOPED_LOCK(push_pop_); + PlatformThread::Sleep(5000); + } + + int pop() { + DFAKE_SCOPED_LOCK(push_pop_); + return 0; + } + + private: + DFAKE_MUTEX(push_pop_); + + DISALLOW_COPY_AND_ASSIGN(NonThreadSafeQueue); + }; + + // This time the QueueUser class protects the non thread safe queue with + // a lock. + class QueueUser : public base::DelegateSimpleThread::Delegate { + public: + QueueUser(NonThreadSafeQueue& queue, Lock& lock) + : queue_(queue), + lock_(lock) {} + + virtual void Run() { + { + AutoLock auto_lock(lock_); + queue_.push(0); + } + { + AutoLock auto_lock(lock_); + queue_.pop(); + } + } + private: + NonThreadSafeQueue& queue_; + Lock& lock_; + }; + + AssertReporter* local_reporter = new AssertReporter(); + + NonThreadSafeQueue queue(local_reporter); + + Lock lock; + + QueueUser queue_user_a(queue, lock); + QueueUser queue_user_b(queue, lock); + + base::DelegateSimpleThread thread_a(&queue_user_a, "queue_user_thread_a"); + base::DelegateSimpleThread thread_b(&queue_user_b, "queue_user_thread_b"); + + thread_a.Start(); + thread_b.Start(); + + thread_a.Join(); + thread_b.Join(); + + EXPECT_FALSE(local_reporter->fail_state()); +} + +TEST(ThreadCollisionTest, MTSynchedScopedRecursiveBookCriticalSectionTest) { + // Queue with a 5 seconds push execution time, hopefuly the two used threads + // in the test will enter the push at same time. + class NonThreadSafeQueue { + public: + explicit NonThreadSafeQueue(base::AsserterBase* asserter) + : push_pop_(asserter) { } + + void push(int) { + DFAKE_SCOPED_RECURSIVE_LOCK(push_pop_); + bar(); + PlatformThread::Sleep(5000); + } + + int pop() { + DFAKE_SCOPED_RECURSIVE_LOCK(push_pop_); + return 0; + } + + void bar() { + DFAKE_SCOPED_RECURSIVE_LOCK(push_pop_); + } + + private: + DFAKE_MUTEX(push_pop_); + + DISALLOW_COPY_AND_ASSIGN(NonThreadSafeQueue); + }; + + // This time the QueueUser class protects the non thread safe queue with + // a lock. + class QueueUser : public base::DelegateSimpleThread::Delegate { + public: + QueueUser(NonThreadSafeQueue& queue, Lock& lock) + : queue_(queue), + lock_(lock) {} + + virtual void Run() { + { + AutoLock auto_lock(lock_); + queue_.push(0); + } + { + AutoLock auto_lock(lock_); + queue_.bar(); + } + { + AutoLock auto_lock(lock_); + queue_.pop(); + } + } + private: + NonThreadSafeQueue& queue_; + Lock& lock_; + }; + + AssertReporter* local_reporter = new AssertReporter(); + + NonThreadSafeQueue queue(local_reporter); + + Lock lock; + + QueueUser queue_user_a(queue, lock); + QueueUser queue_user_b(queue, lock); + + base::DelegateSimpleThread thread_a(&queue_user_a, "queue_user_thread_a"); + base::DelegateSimpleThread thread_b(&queue_user_b, "queue_user_thread_b"); + + thread_a.Start(); + thread_b.Start(); + + thread_a.Join(); + thread_b.Join(); + + EXPECT_FALSE(local_reporter->fail_state()); +} diff --git a/chrome/browser/printing/print_view_manager.cc b/chrome/browser/printing/print_view_manager.cc index 26ae627..9771389 100644 --- a/chrome/browser/printing/print_view_manager.cc +++ b/chrome/browser/printing/print_view_manager.cc @@ -29,6 +29,9 @@ PrintViewManager::PrintViewManager(WebContents& owner) memset(&print_params_, 0, sizeof(print_params_)); } +PrintViewManager::~PrintViewManager() { +} + void PrintViewManager::Destroy() { DisconnectFromCurrentPrintJob(); } diff --git a/chrome/browser/printing/print_view_manager.h b/chrome/browser/printing/print_view_manager.h index b460f19..21226c4 100644 --- a/chrome/browser/printing/print_view_manager.h +++ b/chrome/browser/printing/print_view_manager.h @@ -24,6 +24,7 @@ class PrintViewManager : public NotificationObserver, public PrintedPagesSource { public: PrintViewManager(WebContents& owner); + virtual ~PrintViewManager(); // Destroys the "Print..." dialog, makes sure the pages are finished rendering // and release the print job. |