diff options
Diffstat (limited to 'base')
-rw-r--r-- | base/memory/weak_ptr.cc | 22 | ||||
-rw-r--r-- | base/memory/weak_ptr.h | 23 | ||||
-rw-r--r-- | base/memory/weak_ptr_unittest.cc | 193 |
3 files changed, 18 insertions, 220 deletions
diff --git a/base/memory/weak_ptr.cc b/base/memory/weak_ptr.cc index 9dec8fd..30c777c 100644 --- a/base/memory/weak_ptr.cc +++ b/base/memory/weak_ptr.cc @@ -7,28 +7,28 @@ namespace base { namespace internal { -WeakReference::Flag::Flag() : is_valid_(true) { +WeakReference::Flag::Flag(Flag** handle) : handle_(handle) { } void WeakReference::Flag::Invalidate() { - // The flag being invalidated with a single ref implies that there are no - // weak pointers in existence. Allow deletion on other thread in this case. - DCHECK(thread_checker_.CalledOnValidThread() || HasOneRef()); - is_valid_ = false; + DCHECK(thread_checker_.CalledOnValidThread()); + handle_ = NULL; } bool WeakReference::Flag::IsValid() const { DCHECK(thread_checker_.CalledOnValidThread()); - return is_valid_; + return handle_ != NULL; } WeakReference::Flag::~Flag() { + if (handle_) + *handle_ = NULL; } WeakReference::WeakReference() { } -WeakReference::WeakReference(const Flag* flag) : flag_(flag) { +WeakReference::WeakReference(Flag* flag) : flag_(flag) { } WeakReference::~WeakReference() { @@ -38,7 +38,7 @@ bool WeakReference::is_valid() const { return flag_ && flag_->IsValid(); } -WeakReferenceOwner::WeakReferenceOwner() { +WeakReferenceOwner::WeakReferenceOwner() : flag_(NULL) { } WeakReferenceOwner::~WeakReferenceOwner() { @@ -46,10 +46,8 @@ WeakReferenceOwner::~WeakReferenceOwner() { } WeakReference WeakReferenceOwner::GetRef() const { - // We also want to reattach to the current thread if all previous references - // have gone away. - if (!HasRefs()) - flag_ = new WeakReference::Flag(); + if (!flag_) + flag_ = new WeakReference::Flag(&flag_); return WeakReference(flag_); } diff --git a/base/memory/weak_ptr.h b/base/memory/weak_ptr.h index bdfc308..abda1af 100644 --- a/base/memory/weak_ptr.h +++ b/base/memory/weak_ptr.h @@ -7,15 +7,6 @@ // bound to the lifetime of the referrers. In other words, this is useful when // reference counting is not a good fit. // -// Thread-safety notes: -// When you get a WeakPtr (from a WeakPtrFactory or SupportsWeakPtr), -// the WeakPtr becomes bound to the current thread. You may only -// dereference the WeakPtr on that thread. However, it is safe to -// destroy the WeakPtr object on another thread. -// Since a WeakPtr object may be destroyed on a background thread, -// querying WeakPtrFactory's HasWeakPtrs() method can be racy. -// -// // A common alternative to weak pointers is to have the shared object hold a // list of all referrers, and then when the shared object is destroyed, it // calls a method on the referrers to tell them to drop their references. This @@ -54,6 +45,8 @@ // dereferencing the Controller back pointer after the Controller has been // destroyed. // +// WARNING: weak pointers are not threadsafe!!! You must only use a WeakPtr +// instance on thread where it was created. #ifndef BASE_MEMORY_WEAK_PTR_H_ #define BASE_MEMORY_WEAK_PTR_H_ @@ -76,7 +69,7 @@ class BASE_EXPORT WeakReference { // via base::WeakPtr::~WeakPtr(). class Flag : public RefCountedThreadSafe<Flag> { public: - Flag(); + explicit Flag(Flag** handle); void Invalidate(); bool IsValid() const; @@ -89,17 +82,17 @@ class BASE_EXPORT WeakReference { ~Flag(); ThreadChecker thread_checker_; - bool is_valid_; + Flag** handle_; }; WeakReference(); - explicit WeakReference(const Flag* flag); + WeakReference(Flag* flag); ~WeakReference(); bool is_valid() const; private: - scoped_refptr<const Flag> flag_; + scoped_refptr<Flag> flag_; }; class BASE_EXPORT WeakReferenceOwner { @@ -110,7 +103,7 @@ class BASE_EXPORT WeakReferenceOwner { WeakReference GetRef() const; bool HasRefs() const { - return flag_.get() && !flag_->HasOneRef(); + return flag_ != NULL; } void Invalidate(); @@ -121,7 +114,7 @@ class BASE_EXPORT WeakReferenceOwner { } private: - mutable scoped_refptr<WeakReference::Flag> flag_; + mutable WeakReference::Flag* flag_; }; // This class simplifies the implementation of WeakPtr's type conversion diff --git a/base/memory/weak_ptr_unittest.cc b/base/memory/weak_ptr_unittest.cc index c244ec0..c1a9526 100644 --- a/base/memory/weak_ptr_unittest.cc +++ b/base/memory/weak_ptr_unittest.cc @@ -6,7 +6,6 @@ #include "base/memory/weak_ptr.h" #include "testing/gtest/include/gtest/gtest.h" #include "base/message_loop.h" -#include "base/synchronization/waitable_event.h" #include "base/threading/thread.h" namespace base { @@ -39,104 +38,6 @@ struct Derived : Base {}; struct Producer : SupportsWeakPtr<Producer> {}; struct Consumer { WeakPtr<Producer> producer; }; -// Helper class to create and destroy weak pointer copies -// and delete objects on a background thread. -class BackgroundThread : public Thread { - public: - BackgroundThread() - : Thread("owner_thread") { - } - - void CreateConsumerFromProducer(Consumer** consumer, Producer* producer) { - WaitableEvent completion(true, false); - message_loop()->PostTask( - FROM_HERE, - NewRunnableFunction(&BackgroundThread::DoCreateFromProducer, - consumer, - producer, - &completion)); - completion.Wait(); - } - - void CreateConsumerFromConsumer(Consumer** consumer, const Consumer* other) { - WaitableEvent completion(true, false); - message_loop()->PostTask( - FROM_HERE, - NewRunnableFunction(&BackgroundThread::DoCreateFromConsumer, - consumer, - other, - &completion)); - completion.Wait(); - } - - void DeleteProducer(Producer* object) { - WaitableEvent completion(true, false); - message_loop()->PostTask( - FROM_HERE, - NewRunnableFunction(&BackgroundThread::DoDeleteProducer, - object, - &completion)); - completion.Wait(); - } - - void DeleteConsumer(Consumer* object) { - WaitableEvent completion(true, false); - message_loop()->PostTask( - FROM_HERE, - NewRunnableFunction(&BackgroundThread::DoDeleteConsumer, - object, - &completion)); - completion.Wait(); - } - - Producer* DeRef(const Consumer* consumer) { - WaitableEvent completion(true, false); - Producer* result = NULL; - message_loop()->PostTask( - FROM_HERE, - NewRunnableFunction(&BackgroundThread::DoDeRef, - consumer, - &result, - &completion)); - completion.Wait(); - return result; - } - - protected: - static void DoCreateFromConsumer(Consumer** consumer, - const Consumer* other, - WaitableEvent* completion) { - *consumer = new Consumer; - **consumer = *other; - completion->Signal(); - } - - static void DoCreateFromProducer(Consumer** consumer, - Producer* producer, - WaitableEvent* completion) { - *consumer = new Consumer; - (*consumer)->producer = producer->AsWeakPtr(); - completion->Signal(); - } - - static void DoDeRef(const Consumer* consumer, - Producer** result, - WaitableEvent* completion) { - *result = consumer->producer.get(); - completion->Signal(); - } - - static void DoDeleteProducer(Producer* object, WaitableEvent* completion) { - delete object; - completion->Signal(); - } - - static void DoDeleteConsumer(Consumer* object, WaitableEvent* completion) { - delete object; - completion->Signal(); - } -}; - } // namespace TEST(WeakPtrTest, Basic) { @@ -247,98 +148,4 @@ TEST(WeakPtrTest, SingleThreaded2) { EXPECT_EQ(&producer, consumer->producer.get()); } -TEST(WeakPtrTest, MoveOwnershipImplicit) { - // Move object ownership to other thread by releasing all weak pointers - // on the original thread first. Establishing weak pointers on a different - // thread after previous pointers have been destroyed implicitly reattaches - // the thread checks. - // - Thread A creates object and weak pointer - // - Thread A deletes the weak pointer - // - Thread B creates weak pointer - // - Thread B derefs weak pointer - // - Thread B deletes object - BackgroundThread thread; - thread.Start(); - Producer* producer = new Producer(); - { - WeakPtr<Producer> weak_ptr = producer->AsWeakPtr(); - } - Consumer* consumer; - thread.CreateConsumerFromProducer(&consumer, producer); - EXPECT_EQ(thread.DeRef(consumer), producer); - thread.DeleteProducer(producer); - thread.DeleteConsumer(consumer); -} - -TEST(WeakPtrTest, MoveOwnershipExplicit) { - // Test that we do not trip any checks if we establish weak references - // on one thread and delete the object on another thread after explicit - // detachment. - // - Thread A creates object - // - Thread B creates weak pointer - // - Thread B releases weak pointer - // - Detach owner from Thread B - // - Thread A destroys object - BackgroundThread thread; - thread.Start(); - Producer producer; - Consumer* consumer; - thread.CreateConsumerFromProducer(&consumer, &producer); - EXPECT_EQ(thread.DeRef(consumer), &producer); - thread.DeleteConsumer(consumer); - producer.DetachFromThread(); -} - -TEST(WeakPtrTest, ThreadARefOutlivesThreadBRef) { - // Originating thread has a WeakPtr that outlives others. - // - Thread A creates WeakPtr<> and passes copy to Thread B - // - Destruct the pointer on Thread B - // - Destruct the pointer on Thread A - BackgroundThread thread; - thread.Start(); - Producer producer; - Consumer consumer; - consumer.producer = producer.AsWeakPtr(); - Consumer* consumer_copy; - thread.CreateConsumerFromConsumer(&consumer_copy, &consumer); - EXPECT_EQ(consumer_copy->producer, &producer); - thread.DeleteConsumer(consumer_copy); -} - -TEST(WeakPtrTest, ThreadBRefOutlivesThreadARef) { - // Originating thread drops all references before another thread. - // - Thread A creates WeakPtr<> and passes copy to Thread B - // - Destruct the pointer on Thread A - // - Destruct the pointer on Thread B - BackgroundThread thread; - thread.Start(); - Producer producer; - Consumer* consumer_copy; - { - Consumer consumer; - consumer.producer = producer.AsWeakPtr(); - thread.CreateConsumerFromConsumer(&consumer_copy, &consumer); - } - EXPECT_EQ(consumer_copy->producer, &producer); - thread.DeleteConsumer(consumer_copy); -} - -TEST(WeakPtrTest, OwnerThreadDeletesObject) { - // Originating thread invalidates WeakPtrs while its held by other thread. - // - Thread A creates WeakPtr<> and passes Copy to Thread B - // - WeakReferenceOwner gets destroyed on Thread A - // - WeakPtr gets destroyed on Thread B - BackgroundThread thread; - thread.Start(); - Consumer* consumer_copy; - { - Producer producer; - Consumer consumer; - consumer.producer = producer.AsWeakPtr(); - thread.CreateConsumerFromConsumer(&consumer_copy, &consumer); - } - EXPECT_TRUE(consumer_copy->producer == NULL); - thread.DeleteConsumer(consumer_copy); -} - } // namespace base |