Make WeakPtr thread-safe, i.e. allow cross-thread copying of WeakPtr

and destruction on a thread other than the one where the original
reference was created.

The problem with the current implementation is modifying the flag pointer
from WeakPtr::~WeakPtr which might happen on a different thread (potentially
racing with somebody invalidating the flag on the original thread).

For compatibility reasons, creating the initial reference attaches to the
thread and governs the thread-safety checking logic with respect to checking
a reference to be valid and invalidating it, which should both only be done
on the same thread.

BUG=82509
TEST=added unit tests

Review URL: http://codereview.chromium.org/7677028

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@97808 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 193 insertions, 0 deletions

diff --git a/base/memory/weak_ptr_unittest.cc b/base/memory/weak_ptr_unittest.cc
index c1a9526..c244ec0 100644
--- a/base/memory/weak_ptr_unittest.cc
+++ b/base/memory/weak_ptr_unittest.cc
@@ -6,6 +6,7 @@
 #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 {
@@ -38,6 +39,104 @@ 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) {
@@ -148,4 +247,98 @@ 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