diff options
Diffstat (limited to 'base/synchronization')
| -rw-r--r-- | base/synchronization/atomic_flag.cc | 32 | ||||
| -rw-r--r-- | base/synchronization/atomic_flag.h | 44 | ||||
| -rw-r--r-- | base/synchronization/atomic_flag_unittest.cc | 131 | ||||
| -rw-r--r-- | base/synchronization/cancellation_flag.cc | 26 | ||||
| -rw-r--r-- | base/synchronization/cancellation_flag.h | 41 | ||||
| -rw-r--r-- | base/synchronization/cancellation_flag_unittest.cc | 65 | ||||
| -rw-r--r-- | base/synchronization/condition_variable.h | 6 | ||||
| -rw-r--r-- | base/synchronization/condition_variable_posix.cc | 2 | ||||
| -rw-r--r-- | base/synchronization/lock.h | 17 | ||||
| -rw-r--r-- | base/synchronization/lock_impl.h | 5 | ||||
| -rw-r--r-- | base/synchronization/lock_impl_posix.cc | 55 | ||||
| -rw-r--r-- | base/synchronization/waitable_event.h | 14 | ||||
| -rw-r--r-- | base/synchronization/waitable_event_posix.cc | 20 | ||||
| -rw-r--r-- | base/synchronization/waitable_event_unittest.cc | 57 | ||||
| -rw-r--r-- | base/synchronization/waitable_event_watcher.h | 57 | ||||
| -rw-r--r-- | base/synchronization/waitable_event_watcher_posix.cc | 135 |
16 files changed, 263 insertions, 444 deletions
diff --git a/base/synchronization/atomic_flag.cc b/base/synchronization/atomic_flag.cc deleted file mode 100644 index 8c2018d369..0000000000 --- a/base/synchronization/atomic_flag.cc +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) 2011 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/synchronization/atomic_flag.h" - -#include "base/logging.h" - -namespace base { - -AtomicFlag::AtomicFlag() { - // It doesn't matter where the AtomicFlag is built so long as it's always - // Set() from the same sequence after. Note: the sequencing requirements are - // necessary for IsSet()'s callers to know which sequence's memory operations - // they are synchronized with. - set_sequence_checker_.DetachFromSequence(); -} - -void AtomicFlag::Set() { - DCHECK(set_sequence_checker_.CalledOnValidSequence()); - base::subtle::Release_Store(&flag_, 1); -} - -bool AtomicFlag::IsSet() const { - return base::subtle::Acquire_Load(&flag_) != 0; -} - -void AtomicFlag::UnsafeResetForTesting() { - base::subtle::Release_Store(&flag_, 0); -} - -} // namespace base diff --git a/base/synchronization/atomic_flag.h b/base/synchronization/atomic_flag.h deleted file mode 100644 index ff175e190c..0000000000 --- a/base/synchronization/atomic_flag.h +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (c) 2011 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 BASE_SYNCHRONIZATION_ATOMIC_FLAG_H_ -#define BASE_SYNCHRONIZATION_ATOMIC_FLAG_H_ - -#include "base/atomicops.h" -#include "base/base_export.h" -#include "base/macros.h" -#include "base/sequence_checker.h" - -namespace base { - -// A flag that can safely be set from one thread and read from other threads. -// -// This class IS NOT intended for synchronization between threads. -class BASE_EXPORT AtomicFlag { - public: - AtomicFlag(); - ~AtomicFlag() = default; - - // Set the flag. Must always be called from the same sequence. - void Set(); - - // Returns true iff the flag was set. If this returns true, the current thread - // is guaranteed to be synchronized with all memory operations on the sequence - // which invoked Set() up until at least the first call to Set() on it. - bool IsSet() const; - - // Resets the flag. Be careful when using this: callers might not expect - // IsSet() to return false after returning true once. - void UnsafeResetForTesting(); - - private: - base::subtle::Atomic32 flag_ = 0; - SequenceChecker set_sequence_checker_; - - DISALLOW_COPY_AND_ASSIGN(AtomicFlag); -}; - -} // namespace base - -#endif // BASE_SYNCHRONIZATION_ATOMIC_FLAG_H_ diff --git a/base/synchronization/atomic_flag_unittest.cc b/base/synchronization/atomic_flag_unittest.cc deleted file mode 100644 index a3aa3341a0..0000000000 --- a/base/synchronization/atomic_flag_unittest.cc +++ /dev/null @@ -1,131 +0,0 @@ -// Copyright (c) 2011 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/synchronization/atomic_flag.h" - -#include "base/bind.h" -#include "base/logging.h" -#include "base/single_thread_task_runner.h" -#include "base/synchronization/waitable_event.h" -#include "base/test/gtest_util.h" -#include "base/threading/platform_thread.h" -#include "base/threading/thread.h" -#include "build/build_config.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace base { - -namespace { - -void ExpectSetFlagDeath(AtomicFlag* flag) { - ASSERT_TRUE(flag); - EXPECT_DCHECK_DEATH(flag->Set()); -} - -// Busy waits (to explicitly avoid using synchronization constructs that would -// defeat the purpose of testing atomics) until |tested_flag| is set and then -// verifies that non-atomic |*expected_after_flag| is true and sets |*done_flag| -// before returning if it's non-null. -void BusyWaitUntilFlagIsSet(AtomicFlag* tested_flag, bool* expected_after_flag, - AtomicFlag* done_flag) { - while (!tested_flag->IsSet()) - PlatformThread::YieldCurrentThread(); - - EXPECT_TRUE(*expected_after_flag); - if (done_flag) - done_flag->Set(); -} - -} // namespace - -TEST(AtomicFlagTest, SimpleSingleThreadedTest) { - AtomicFlag flag; - ASSERT_FALSE(flag.IsSet()); - flag.Set(); - ASSERT_TRUE(flag.IsSet()); -} - -TEST(AtomicFlagTest, DoubleSetTest) { - AtomicFlag flag; - ASSERT_FALSE(flag.IsSet()); - flag.Set(); - ASSERT_TRUE(flag.IsSet()); - flag.Set(); - ASSERT_TRUE(flag.IsSet()); -} - -TEST(AtomicFlagTest, ReadFromDifferentThread) { - // |tested_flag| is the one being tested below. - AtomicFlag tested_flag; - // |expected_after_flag| is used to confirm that sequential consistency is - // obtained around |tested_flag|. - bool expected_after_flag = false; - // |reset_flag| is used to confirm the test flows as intended without using - // synchronization constructs which would defeat the purpose of exercising - // atomics. - AtomicFlag reset_flag; - - Thread thread("AtomicFlagTest.ReadFromDifferentThread"); - ASSERT_TRUE(thread.Start()); - thread.task_runner()->PostTask( - FROM_HERE, - Bind(&BusyWaitUntilFlagIsSet, &tested_flag, &expected_after_flag, - &reset_flag)); - - // To verify that IsSet() fetches the flag's value from memory every time it - // is called (not just the first time that it is called on a thread), sleep - // before setting the flag. - PlatformThread::Sleep(TimeDelta::FromMilliseconds(20)); - - // |expected_after_flag| is used to verify that all memory operations - // performed before |tested_flag| is Set() are visible to threads that can see - // IsSet(). - expected_after_flag = true; - tested_flag.Set(); - - // Sleep again to give the busy loop time to observe the flag and verify - // expectations. - PlatformThread::Sleep(TimeDelta::FromMilliseconds(20)); - - // Use |reset_flag| to confirm that the above completed (which the rest of - // this test assumes). - ASSERT_TRUE(reset_flag.IsSet()); - - tested_flag.UnsafeResetForTesting(); - EXPECT_FALSE(tested_flag.IsSet()); - expected_after_flag = false; - - // Perform the same test again after the controlled UnsafeResetForTesting(), - // |thread| is guaranteed to be synchronized past the - // |UnsafeResetForTesting()| call when the task runs per the implicit - // synchronization in the post task mechanism. - thread.task_runner()->PostTask( - FROM_HERE, - Bind(&BusyWaitUntilFlagIsSet, &tested_flag, &expected_after_flag, - nullptr)); - - PlatformThread::Sleep(TimeDelta::FromMilliseconds(20)); - - expected_after_flag = true; - tested_flag.Set(); - - // The |thread|'s destructor will block until the posted task completes, so - // the test will time out if it fails to see the flag be set. -} - -TEST(AtomicFlagTest, SetOnDifferentSequenceDeathTest) { - // Checks that Set() can't be called from another sequence after being called - // on this one. AtomicFlag should die on a DCHECK if Set() is called again - // from another sequence. - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - Thread t("AtomicFlagTest.SetOnDifferentThreadDeathTest"); - ASSERT_TRUE(t.Start()); - EXPECT_TRUE(t.WaitUntilThreadStarted()); - - AtomicFlag flag; - flag.Set(); - t.task_runner()->PostTask(FROM_HERE, Bind(&ExpectSetFlagDeath, &flag)); -} - -} // namespace base diff --git a/base/synchronization/cancellation_flag.cc b/base/synchronization/cancellation_flag.cc new file mode 100644 index 0000000000..ca5c0a8283 --- /dev/null +++ b/base/synchronization/cancellation_flag.cc @@ -0,0 +1,26 @@ +// Copyright (c) 2011 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/synchronization/cancellation_flag.h" + +#include "base/logging.h" + +namespace base { + +void CancellationFlag::Set() { +#if !defined(NDEBUG) + DCHECK_EQ(set_on_, PlatformThread::CurrentId()); +#endif + base::subtle::Release_Store(&flag_, 1); +} + +bool CancellationFlag::IsSet() const { + return base::subtle::Acquire_Load(&flag_) != 0; +} + +void CancellationFlag::UnsafeResetForTesting() { + base::subtle::Release_Store(&flag_, 0); +} + +} // namespace base diff --git a/base/synchronization/cancellation_flag.h b/base/synchronization/cancellation_flag.h index 39094e2dc0..f2f83f47da 100644 --- a/base/synchronization/cancellation_flag.h +++ b/base/synchronization/cancellation_flag.h @@ -5,15 +5,44 @@ #ifndef BASE_SYNCHRONIZATION_CANCELLATION_FLAG_H_ #define BASE_SYNCHRONIZATION_CANCELLATION_FLAG_H_ -#include "base/synchronization/atomic_flag.h" +#include "base/atomicops.h" +#include "base/base_export.h" +#include "base/macros.h" +#include "base/threading/platform_thread.h" namespace base { -// Use inheritance instead of "using" to allow forward declaration of "class -// CancellationFlag". -// TODO(fdoray): Replace CancellationFlag with AtomicFlag throughout the -// codebase and delete this file. crbug.com/630251 -class CancellationFlag : public AtomicFlag {}; +// CancellationFlag allows one thread to cancel jobs executed on some worker +// thread. Calling Set() from one thread and IsSet() from a number of threads +// is thread-safe. +// +// This class IS NOT intended for synchronization between threads. +class BASE_EXPORT CancellationFlag { + public: + CancellationFlag() : flag_(false) { +#if !defined(NDEBUG) + set_on_ = PlatformThread::CurrentId(); +#endif + } + ~CancellationFlag() {} + + // Set the flag. May only be called on the thread which owns the object. + void Set(); + bool IsSet() const; // Returns true iff the flag was set. + + // For subtle reasons that may be different on different architectures, + // a different thread testing IsSet() may erroneously read 'true' after + // this method has been called. + void UnsafeResetForTesting(); + + private: + base::subtle::Atomic32 flag_; +#if !defined(NDEBUG) + PlatformThreadId set_on_; +#endif + + DISALLOW_COPY_AND_ASSIGN(CancellationFlag); +}; } // namespace base diff --git a/base/synchronization/cancellation_flag_unittest.cc b/base/synchronization/cancellation_flag_unittest.cc new file mode 100644 index 0000000000..13c74bcbd4 --- /dev/null +++ b/base/synchronization/cancellation_flag_unittest.cc @@ -0,0 +1,65 @@ +// Copyright (c) 2011 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. + +// Tests of CancellationFlag class. + +#include "base/synchronization/cancellation_flag.h" + +#include "base/bind.h" +#include "base/location.h" +#include "base/logging.h" +#include "base/single_thread_task_runner.h" +#include "base/synchronization/spin_wait.h" +#include "base/threading/thread.h" +#include "base/time/time.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "testing/platform_test.h" + +namespace base { + +namespace { + +//------------------------------------------------------------------------------ +// Define our test class. +//------------------------------------------------------------------------------ + +void CancelHelper(CancellationFlag* flag) { +#if GTEST_HAS_DEATH_TEST + ASSERT_DEBUG_DEATH(flag->Set(), ""); +#endif +} + +TEST(CancellationFlagTest, SimpleSingleThreadedTest) { + CancellationFlag flag; + ASSERT_FALSE(flag.IsSet()); + flag.Set(); + ASSERT_TRUE(flag.IsSet()); +} + +TEST(CancellationFlagTest, DoubleSetTest) { + CancellationFlag flag; + ASSERT_FALSE(flag.IsSet()); + flag.Set(); + ASSERT_TRUE(flag.IsSet()); + flag.Set(); + ASSERT_TRUE(flag.IsSet()); +} + +TEST(CancellationFlagTest, SetOnDifferentThreadDeathTest) { + // Checks that Set() can't be called from any other thread. + // CancellationFlag should die on a DCHECK if Set() is called from + // other thread. + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + Thread t("CancellationFlagTest.SetOnDifferentThreadDeathTest"); + ASSERT_TRUE(t.Start()); + ASSERT_TRUE(t.message_loop()); + ASSERT_TRUE(t.IsRunning()); + + CancellationFlag flag; + t.task_runner()->PostTask(FROM_HERE, base::Bind(&CancelHelper, &flag)); +} + +} // namespace + +} // namespace base diff --git a/base/synchronization/condition_variable.h b/base/synchronization/condition_variable.h index b567751172..ebf90d249a 100644 --- a/base/synchronization/condition_variable.h +++ b/base/synchronization/condition_variable.h @@ -91,13 +91,11 @@ class BASE_EXPORT ConditionVariable { ~ConditionVariable(); // Wait() releases the caller's critical section atomically as it starts to - // sleep, and the reacquires it when it is signaled. The wait functions are - // susceptible to spurious wakeups. (See usage note 1 for more details.) + // sleep, and the reacquires it when it is signaled. void Wait(); void TimedWait(const TimeDelta& max_time); - // Broadcast() revives all waiting threads. (See usage note 2 for more - // details.) + // Broadcast() revives all waiting threads. void Broadcast(); // Signal() revives one waiting thread. void Signal(); diff --git a/base/synchronization/condition_variable_posix.cc b/base/synchronization/condition_variable_posix.cc index d07c671810..d86fd180ec 100644 --- a/base/synchronization/condition_variable_posix.cc +++ b/base/synchronization/condition_variable_posix.cc @@ -118,8 +118,6 @@ void ConditionVariable::TimedWait(const TimeDelta& max_time) { #endif // OS_ANDROID && HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC #endif // OS_MACOSX - // On failure, we only expect the CV to timeout. Any other error value means - // that we've unexpectedly woken up. DCHECK(rv == 0 || rv == ETIMEDOUT); #if DCHECK_IS_ON() user_lock_->CheckUnheldAndMark(); diff --git a/base/synchronization/lock.h b/base/synchronization/lock.h index 599984e8b6..fbf6cef769 100644 --- a/base/synchronization/lock.h +++ b/base/synchronization/lock.h @@ -61,23 +61,6 @@ class BASE_EXPORT Lock { void AssertAcquired() const; #endif // DCHECK_IS_ON() - // Whether Lock mitigates priority inversion when used from different thread - // priorities. - static bool HandlesMultipleThreadPriorities() { -#if defined(OS_POSIX) - // POSIX mitigates priority inversion by setting the priority of a thread - // holding a Lock to the maximum priority of any other thread waiting on it. - return internal::LockImpl::PriorityInheritanceAvailable(); -#elif defined(OS_WIN) - // Windows mitigates priority inversion by randomly boosting the priority of - // ready threads. - // https://msdn.microsoft.com/library/windows/desktop/ms684831.aspx - return true; -#else -#error Unsupported platform -#endif - } - #if defined(OS_POSIX) || defined(OS_WIN) // Both Windows and POSIX implementations of ConditionVariable need to be // able to see our lock and tweak our debugging counters, as they release and diff --git a/base/synchronization/lock_impl.h b/base/synchronization/lock_impl.h index 603585a050..cbaabc784b 100644 --- a/base/synchronization/lock_impl.h +++ b/base/synchronization/lock_impl.h @@ -48,11 +48,6 @@ class BASE_EXPORT LockImpl { // unnecessary. NativeHandle* native_handle() { return &native_handle_; } -#if defined(OS_POSIX) - // Whether this lock will attempt to use priority inheritance. - static bool PriorityInheritanceAvailable(); -#endif - private: NativeHandle native_handle_; diff --git a/base/synchronization/lock_impl_posix.cc b/base/synchronization/lock_impl_posix.cc index ff997ea65f..5619adaf5d 100644 --- a/base/synchronization/lock_impl_posix.cc +++ b/base/synchronization/lock_impl_posix.cc @@ -7,45 +7,27 @@ #include <errno.h> #include <string.h> -#include "base/debug/activity_tracker.h" #include "base/logging.h" -#include "base/synchronization/lock.h" namespace base { namespace internal { -// Determines which platforms can consider using priority inheritance locks. Use -// this define for platform code that may not compile if priority inheritance -// locks aren't available. For this platform code, -// PRIORITY_INHERITANCE_LOCKS_POSSIBLE() is a necessary but insufficient check. -// Lock::PriorityInheritanceAvailable still must be checked as the code may -// compile but the underlying platform still may not correctly support priority -// inheritance locks. -#if defined(OS_NACL) || defined(OS_ANDROID) || defined(__ANDROID__) -#define PRIORITY_INHERITANCE_LOCKS_POSSIBLE() 0 -#else -#define PRIORITY_INHERITANCE_LOCKS_POSSIBLE() 1 -#endif - LockImpl::LockImpl() { +#ifndef NDEBUG + // In debug, setup attributes for lock error checking. pthread_mutexattr_t mta; int rv = pthread_mutexattr_init(&mta); DCHECK_EQ(rv, 0) << ". " << strerror(rv); -#if PRIORITY_INHERITANCE_LOCKS_POSSIBLE() - if (PriorityInheritanceAvailable()) { - rv = pthread_mutexattr_setprotocol(&mta, PTHREAD_PRIO_INHERIT); - DCHECK_EQ(rv, 0) << ". " << strerror(rv); - } -#endif -#ifndef NDEBUG - // In debug, setup attributes for lock error checking. rv = pthread_mutexattr_settype(&mta, PTHREAD_MUTEX_ERRORCHECK); DCHECK_EQ(rv, 0) << ". " << strerror(rv); -#endif rv = pthread_mutex_init(&native_handle_, &mta); DCHECK_EQ(rv, 0) << ". " << strerror(rv); rv = pthread_mutexattr_destroy(&mta); DCHECK_EQ(rv, 0) << ". " << strerror(rv); +#else + // In release, go with the default lock attributes. + pthread_mutex_init(&native_handle_, NULL); +#endif } LockImpl::~LockImpl() { @@ -60,7 +42,6 @@ bool LockImpl::Try() { } void LockImpl::Lock() { - base::debug::ScopedLockAcquireActivity lock_activity(this); int rv = pthread_mutex_lock(&native_handle_); DCHECK_EQ(rv, 0) << ". " << strerror(rv); } @@ -70,29 +51,5 @@ void LockImpl::Unlock() { DCHECK_EQ(rv, 0) << ". " << strerror(rv); } -// static -bool LockImpl::PriorityInheritanceAvailable() { -#if PRIORITY_INHERITANCE_LOCKS_POSSIBLE() && defined(OS_MACOSX) - return true; -#else - // Security concerns prevent the use of priority inheritance mutexes on Linux. - // * CVE-2010-0622 - wake_futex_pi unlocks incorrect, possible DoS. - // https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2010-0622 - // * CVE-2012-6647 - Linux < 3.5.1, futex_wait_requeue_pi possible DoS. - // https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2012-6647 - // * CVE-2014-3153 - Linux <= 3.14.5, futex_requeue, privilege escalation. - // https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-3153 - // - // If the above were all addressed, we still need a runtime check to deal with - // the bug below. - // * glibc Bug 14652: https://sourceware.org/bugzilla/show_bug.cgi?id=14652 - // Fixed in glibc 2.17. - // Priority inheritance mutexes may deadlock with condition variables - // during recacquisition of the mutex after the condition variable is - // signalled. - return false; -#endif -} - } // namespace internal } // namespace base diff --git a/base/synchronization/waitable_event.h b/base/synchronization/waitable_event.h index 761965f03a..3863e98455 100644 --- a/base/synchronization/waitable_event.h +++ b/base/synchronization/waitable_event.h @@ -25,7 +25,6 @@ namespace base { class TimeDelta; -class TimeTicks; // A WaitableEvent can be a useful thread synchronization tool when you want to // allow one thread to wait for another thread to finish some work. For @@ -87,17 +86,12 @@ class BASE_EXPORT WaitableEvent { // delete e; void Wait(); - // Wait up until wait_delta has passed for the event to be signaled. Returns - // true if the event was signaled. + // Wait up until max_time has passed for the event to be signaled. Returns + // true if the event was signaled. If this method returns false, then it + // does not necessarily mean that max_time was exceeded. // // TimedWait can synchronise its own destruction like |Wait|. - bool TimedWait(const TimeDelta& wait_delta); - - // Wait up until end_time deadline has passed for the event to be signaled. - // Return true if the event was signaled. - // - // TimedWaitUntil can synchronise its own destruction like |Wait|. - bool TimedWaitUntil(const TimeTicks& end_time); + bool TimedWait(const TimeDelta& max_time); #if defined(OS_WIN) HANDLE handle() const { return handle_.Get(); } diff --git a/base/synchronization/waitable_event_posix.cc b/base/synchronization/waitable_event_posix.cc index 5dfff468ad..b32c882711 100644 --- a/base/synchronization/waitable_event_posix.cc +++ b/base/synchronization/waitable_event_posix.cc @@ -7,7 +7,6 @@ #include <algorithm> #include <vector> -#include "base/debug/activity_tracker.h" #include "base/logging.h" #include "base/synchronization/condition_variable.h" #include "base/synchronization/lock.h" @@ -153,22 +152,14 @@ class SyncWaiter : public WaitableEvent::Waiter { }; void WaitableEvent::Wait() { - bool result = TimedWaitUntil(TimeTicks::Max()); + bool result = TimedWait(TimeDelta::FromSeconds(-1)); DCHECK(result) << "TimedWait() should never fail with infinite timeout"; } -bool WaitableEvent::TimedWait(const TimeDelta& wait_delta) { - // TimeTicks takes care of overflow including the cases when wait_delta - // is a maximum value. - return TimedWaitUntil(TimeTicks::Now() + wait_delta); -} - -bool WaitableEvent::TimedWaitUntil(const TimeTicks& end_time) { +bool WaitableEvent::TimedWait(const TimeDelta& max_time) { base::ThreadRestrictions::AssertWaitAllowed(); - // Record the event that this thread is blocking upon (for hang diagnosis). - base::debug::ScopedEventWaitActivity event_activity(this); - - const bool finite_time = !end_time.is_max(); + const TimeTicks end_time(TimeTicks::Now() + max_time); + const bool finite_time = max_time.ToInternalValue() >= 0; kernel_->lock_.Acquire(); if (kernel_->signaled_) { @@ -241,9 +232,6 @@ size_t WaitableEvent::WaitMany(WaitableEvent** raw_waitables, base::ThreadRestrictions::AssertWaitAllowed(); DCHECK(count) << "Cannot wait on no events"; - // Record an event (the first) that this thread is blocking upon. - base::debug::ScopedEventWaitActivity event_activity(raw_waitables[0]); - // We need to acquire the locks in a globally consistent order. Thus we sort // the array of waitables by address. We actually sort a pairs so that we can // map back to the original index values later. diff --git a/base/synchronization/waitable_event_unittest.cc b/base/synchronization/waitable_event_unittest.cc index c0e280aa97..ac5c9f1255 100644 --- a/base/synchronization/waitable_event_unittest.cc +++ b/base/synchronization/waitable_event_unittest.cc @@ -136,7 +136,13 @@ TEST(WaitableEventTest, WaitMany) { // Tests that using TimeDelta::Max() on TimedWait() is not the same as passing // a timeout of 0. (crbug.com/465948) -TEST(WaitableEventTest, TimedWait) { +#if defined(OS_POSIX) +// crbug.com/465948 not fixed yet. +#define MAYBE_TimedWait DISABLED_TimedWait +#else +#define MAYBE_TimedWait TimedWait +#endif +TEST(WaitableEventTest, MAYBE_TimedWait) { WaitableEvent* ev = new WaitableEvent(WaitableEvent::ResetPolicy::AUTOMATIC, WaitableEvent::InitialState::NOT_SIGNALED); @@ -147,58 +153,11 @@ TEST(WaitableEventTest, TimedWait) { TimeTicks start = TimeTicks::Now(); PlatformThread::Create(0, &signaler, &thread); - EXPECT_TRUE(ev->TimedWait(TimeDelta::Max())); + ev->TimedWait(TimeDelta::Max()); EXPECT_GE(TimeTicks::Now() - start, thread_delay); delete ev; PlatformThread::Join(thread); } -// Tests that a sub-ms TimedWait doesn't time out promptly. -TEST(WaitableEventTest, SubMsTimedWait) { - WaitableEvent ev(WaitableEvent::ResetPolicy::AUTOMATIC, - WaitableEvent::InitialState::NOT_SIGNALED); - - TimeDelta delay = TimeDelta::FromMicroseconds(900); - TimeTicks start_time = TimeTicks::Now(); - ev.TimedWait(delay); - EXPECT_GE(TimeTicks::Now() - start_time, delay); -} - -// Tests that TimedWaitUntil can be safely used with various end_time deadline -// values. -TEST(WaitableEventTest, TimedWaitUntil) { - WaitableEvent ev(WaitableEvent::ResetPolicy::AUTOMATIC, - WaitableEvent::InitialState::NOT_SIGNALED); - - TimeTicks start_time(TimeTicks::Now()); - TimeDelta delay = TimeDelta::FromMilliseconds(10); - - // Should be OK to wait for the current time or time in the past. - // That should end promptly and be equivalent to IsSignalled. - EXPECT_FALSE(ev.TimedWaitUntil(start_time)); - EXPECT_FALSE(ev.TimedWaitUntil(start_time - delay)); - - // Should be OK to wait for zero TimeTicks(). - EXPECT_FALSE(ev.TimedWaitUntil(TimeTicks())); - - // Waiting for a time in the future shouldn't end before the deadline - // if the event isn't signalled. - EXPECT_FALSE(ev.TimedWaitUntil(start_time + delay)); - EXPECT_GE(TimeTicks::Now() - start_time, delay); - - // Test that passing TimeTicks::Max to TimedWaitUntil is valid and isn't - // the same as passing TimeTicks(). Also verifies that signaling event - // ends the wait promptly. - WaitableEventSignaler signaler(delay, &ev); - PlatformThreadHandle thread; - start_time = TimeTicks::Now(); - PlatformThread::Create(0, &signaler, &thread); - - EXPECT_TRUE(ev.TimedWaitUntil(TimeTicks::Max())); - EXPECT_GE(TimeTicks::Now() - start_time, delay); - - PlatformThread::Join(thread); -} - } // namespace base diff --git a/base/synchronization/waitable_event_watcher.h b/base/synchronization/waitable_event_watcher.h index 44ef5047ed..eb51effa49 100644 --- a/base/synchronization/waitable_event_watcher.h +++ b/base/synchronization/waitable_event_watcher.h @@ -6,14 +6,13 @@ #define BASE_SYNCHRONIZATION_WAITABLE_EVENT_WATCHER_H_ #include "base/base_export.h" -#include "base/macros.h" -#include "base/sequence_checker.h" #include "build/build_config.h" #if defined(OS_WIN) #include "base/win/object_watcher.h" #else #include "base/callback.h" +#include "base/message_loop/message_loop.h" #include "base/synchronization/waitable_event.h" #endif @@ -21,13 +20,14 @@ namespace base { class Flag; class AsyncWaiter; +class AsyncCallbackTask; class WaitableEvent; // This class provides a way to wait on a WaitableEvent asynchronously. // // Each instance of this object can be waiting on a single WaitableEvent. When -// the waitable event is signaled, a callback is invoked on the sequence that -// called StartWatching(). This callback can be deleted by deleting the waiter. +// the waitable event is signaled, a callback is made in the thread of a given +// MessageLoop. This callback can be deleted by deleting the waiter. // // Typical usage: // @@ -60,56 +60,53 @@ class WaitableEvent; class BASE_EXPORT WaitableEventWatcher #if defined(OS_WIN) - : public win::ObjectWatcher::Delegate + : public win::ObjectWatcher::Delegate { +#else + : public MessageLoop::DestructionObserver { #endif -{ public: typedef Callback<void(WaitableEvent*)> EventCallback; WaitableEventWatcher(); - -#if defined(OS_WIN) ~WaitableEventWatcher() override; -#else - ~WaitableEventWatcher(); -#endif - // When |event| is signaled, |callback| is called on the sequence that called - // StartWatching(). + // When @event is signaled, the given callback is called on the thread of the + // current message loop when StartWatching is called. bool StartWatching(WaitableEvent* event, const EventCallback& callback); - // Cancel the current watch. Must be called from the same sequence which + // Cancel the current watch. Must be called from the same thread which // started the watch. // // Does nothing if no event is being watched, nor if the watch has completed. // The callback will *not* be called for the current watch after this - // function returns. Since the callback runs on the same sequence as this + // function returns. Since the callback runs on the same thread as this // function, it cannot be called during this function either. void StopWatching(); + // Return the currently watched event, or NULL if no object is currently being + // watched. + WaitableEvent* GetWatchedEvent(); + + // Return the callback that will be invoked when the event is + // signaled. + const EventCallback& callback() const { return callback_; } + private: #if defined(OS_WIN) void OnObjectSignaled(HANDLE h) override; - win::ObjectWatcher watcher_; - EventCallback callback_; - WaitableEvent* event_ = nullptr; #else - // Instantiated in StartWatching(). Set before the callback runs. Reset in - // StopWatching() or StartWatching(). - scoped_refptr<Flag> cancel_flag_; - - // Enqueued in the wait list of the watched WaitableEvent. - AsyncWaiter* waiter_ = nullptr; + // Implementation of MessageLoop::DestructionObserver + void WillDestroyCurrentMessageLoop() override; - // Kernel of the watched WaitableEvent. + MessageLoop* message_loop_; + scoped_refptr<Flag> cancel_flag_; + AsyncWaiter* waiter_; + base::Closure internal_callback_; scoped_refptr<WaitableEvent::WaitableEventKernel> kernel_; - - // Ensures that StartWatching() and StopWatching() are called on the same - // sequence. - SequenceChecker sequence_checker_; #endif - DISALLOW_COPY_AND_ASSIGN(WaitableEventWatcher); + WaitableEvent* event_; + EventCallback callback_; }; } // namespace base diff --git a/base/synchronization/waitable_event_watcher_posix.cc b/base/synchronization/waitable_event_watcher_posix.cc index 3adbc5f977..7cf8688d4c 100644 --- a/base/synchronization/waitable_event_watcher_posix.cc +++ b/base/synchronization/waitable_event_watcher_posix.cc @@ -4,12 +4,12 @@ #include "base/synchronization/waitable_event_watcher.h" -#include <utility> - #include "base/bind.h" -#include "base/logging.h" +#include "base/location.h" +#include "base/macros.h" +#include "base/single_thread_task_runner.h" #include "base/synchronization/lock.h" -#include "base/threading/sequenced_task_runner_handle.h" +#include "base/synchronization/waitable_event.h" namespace base { @@ -17,15 +17,14 @@ namespace base { // WaitableEventWatcher (async waits). // // The basic design is that we add an AsyncWaiter to the wait-list of the event. -// That AsyncWaiter has a pointer to SequencedTaskRunner, and a Task to be -// posted to it. The task ends up calling the callback when it runs on the -// sequence. +// That AsyncWaiter has a pointer to MessageLoop, and a Task to be posted to it. +// The MessageLoop ends up running the task, which calls the delegate. // // Since the wait can be canceled, we have a thread-safe Flag object which is // set when the wait has been canceled. At each stage in the above, we check the // flag before going onto the next stage. Since the wait may only be canceled in -// the sequence which runs the Task, we are assured that the callback cannot be -// called after canceling... +// the MessageLoop which runs the Task, we are assured that the delegate cannot +// be called after canceling... // ----------------------------------------------------------------------------- // A thread-safe, reference-counted, write-once flag. @@ -55,22 +54,23 @@ class Flag : public RefCountedThreadSafe<Flag> { }; // ----------------------------------------------------------------------------- -// This is an asynchronous waiter which posts a task to a SequencedTaskRunner -// when fired. An AsyncWaiter may only be in a single wait-list. +// This is an asynchronous waiter which posts a task to a MessageLoop when +// fired. An AsyncWaiter may only be in a single wait-list. // ----------------------------------------------------------------------------- class AsyncWaiter : public WaitableEvent::Waiter { public: - AsyncWaiter(scoped_refptr<SequencedTaskRunner> task_runner, + AsyncWaiter(MessageLoop* message_loop, const base::Closure& callback, Flag* flag) - : task_runner_(std::move(task_runner)), + : message_loop_(message_loop), callback_(callback), - flag_(flag) {} + flag_(flag) { } bool Fire(WaitableEvent* event) override { // Post the callback if we haven't been cancelled. - if (!flag_->value()) - task_runner_->PostTask(FROM_HERE, callback_); + if (!flag_->value()) { + message_loop_->task_runner()->PostTask(FROM_HERE, callback_); + } // We are removed from the wait-list by the WaitableEvent itself. It only // remains to delete ourselves. @@ -85,37 +85,37 @@ class AsyncWaiter : public WaitableEvent::Waiter { bool Compare(void* tag) override { return tag == flag_.get(); } private: - const scoped_refptr<SequencedTaskRunner> task_runner_; - const base::Closure callback_; - const scoped_refptr<Flag> flag_; + MessageLoop *const message_loop_; + base::Closure callback_; + scoped_refptr<Flag> flag_; }; // ----------------------------------------------------------------------------- -// For async waits we need to run a callback on a sequence. We do this by -// posting an AsyncCallbackHelper task, which calls the callback and keeps track -// of when the event is canceled. +// For async waits we need to make a callback in a MessageLoop thread. We do +// this by posting a callback, which calls the delegate and keeps track of when +// the event is canceled. // ----------------------------------------------------------------------------- void AsyncCallbackHelper(Flag* flag, const WaitableEventWatcher::EventCallback& callback, WaitableEvent* event) { - // Runs on the sequence that called StartWatching(). + // Runs in MessageLoop thread. if (!flag->value()) { - // This is to let the WaitableEventWatcher know that the event has occured. + // This is to let the WaitableEventWatcher know that the event has occured + // because it needs to be able to return NULL from GetWatchedObject flag->Set(); callback.Run(event); } } -WaitableEventWatcher::WaitableEventWatcher() { - sequence_checker_.DetachFromSequence(); +WaitableEventWatcher::WaitableEventWatcher() + : message_loop_(NULL), + cancel_flag_(NULL), + waiter_(NULL), + event_(NULL) { } WaitableEventWatcher::~WaitableEventWatcher() { - // The destructor may be called from a different sequence than StartWatching() - // when there is no active watch. To avoid triggering a DCHECK in - // StopWatching(), do not call it when there is no active watch. - if (cancel_flag_ && !cancel_flag_->value()) - StopWatching(); + StopWatching(); } // ----------------------------------------------------------------------------- @@ -125,44 +125,61 @@ WaitableEventWatcher::~WaitableEventWatcher() { bool WaitableEventWatcher::StartWatching( WaitableEvent* event, const EventCallback& callback) { - DCHECK(sequence_checker_.CalledOnValidSequence()); - DCHECK(SequencedTaskRunnerHandle::Get()); + MessageLoop *const current_ml = MessageLoop::current(); + DCHECK(current_ml) << "Cannot create WaitableEventWatcher without a " + "current MessageLoop"; // A user may call StartWatching from within the callback function. In this // case, we won't know that we have finished watching, expect that the Flag // will have been set in AsyncCallbackHelper(). - if (cancel_flag_.get() && cancel_flag_->value()) - cancel_flag_ = nullptr; + if (cancel_flag_.get() && cancel_flag_->value()) { + if (message_loop_) { + message_loop_->RemoveDestructionObserver(this); + message_loop_ = NULL; + } + + cancel_flag_ = NULL; + } - DCHECK(!cancel_flag_) << "StartWatching called while still watching"; + DCHECK(!cancel_flag_.get()) << "StartWatching called while still watching"; cancel_flag_ = new Flag; - const Closure internal_callback = base::Bind( - &AsyncCallbackHelper, base::RetainedRef(cancel_flag_), callback, event); + callback_ = callback; + internal_callback_ = base::Bind( + &AsyncCallbackHelper, base::RetainedRef(cancel_flag_), callback_, event); WaitableEvent::WaitableEventKernel* kernel = event->kernel_.get(); AutoLock locked(kernel->lock_); + event_ = event; + if (kernel->signaled_) { if (!kernel->manual_reset_) kernel->signaled_ = false; // No hairpinning - we can't call the delegate directly here. We have to - // post a task to the SequencedTaskRunnerHandle as usual. - SequencedTaskRunnerHandle::Get()->PostTask(FROM_HERE, internal_callback); + // enqueue a task on the MessageLoop as normal. + current_ml->task_runner()->PostTask(FROM_HERE, internal_callback_); return true; } + message_loop_ = current_ml; + current_ml->AddDestructionObserver(this); + kernel_ = kernel; - waiter_ = new AsyncWaiter(SequencedTaskRunnerHandle::Get(), internal_callback, - cancel_flag_.get()); + waiter_ = new AsyncWaiter(current_ml, internal_callback_, cancel_flag_.get()); event->Enqueue(waiter_); return true; } void WaitableEventWatcher::StopWatching() { - DCHECK(sequence_checker_.CalledOnValidSequence()); + callback_.Reset(); + + if (message_loop_) { + message_loop_->RemoveDestructionObserver(this); + message_loop_ = NULL; + } if (!cancel_flag_.get()) // if not currently watching... return; @@ -210,24 +227,44 @@ void WaitableEventWatcher::StopWatching() { // have been enqueued with the MessageLoop because the waiter was never // signaled) delete waiter_; + internal_callback_.Reset(); cancel_flag_ = NULL; return; } - // Case 3: the waiter isn't on the wait-list, thus it was signaled. It may not - // have run yet, so we set the flag to tell it not to bother enqueuing the - // task on the SequencedTaskRunner, but to delete it instead. The Waiter - // deletes itself once run. + // Case 3: the waiter isn't on the wait-list, thus it was signaled. It may + // not have run yet, so we set the flag to tell it not to bother enqueuing the + // task on the MessageLoop, but to delete it instead. The Waiter deletes + // itself once run. cancel_flag_->Set(); cancel_flag_ = NULL; // If the waiter has already run then the task has been enqueued. If the Task // hasn't yet run, the flag will stop the delegate from getting called. (This - // is thread safe because one may only delete a Handle from the sequence that - // called StartWatching()). + // is thread safe because one may only delete a Handle from the MessageLoop + // thread.) // // If the delegate has already been called then we have nothing to do. The // task has been deleted by the MessageLoop. } +WaitableEvent* WaitableEventWatcher::GetWatchedEvent() { + if (!cancel_flag_.get()) + return NULL; + + if (cancel_flag_->value()) + return NULL; + + return event_; +} + +// ----------------------------------------------------------------------------- +// This is called when the MessageLoop which the callback will be run it is +// deleted. We need to cancel the callback as if we had been deleted, but we +// will still be deleted at some point in the future. +// ----------------------------------------------------------------------------- +void WaitableEventWatcher::WillDestroyCurrentMessageLoop() { + StopWatching(); +} + } // namespace base |