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

base/memory/weak_ptr_unittest.cc

Index: base/memory/weak_ptr_unittest.cc
diff --git a/base/memory/weak_ptr_unittest.cc b/base/memory/weak_ptr_unittest.cc
index c1a952634fd204615fa782a33b496bf173c13b06..806bbd40b6057f3eb7f2dbe17677625c5b05f63b 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,105 @@ 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 : protected Thread {
+ public:
+  BackgroundThread()
+      : Thread("owner_thread") {
+    Start();

[awong, 2011/08/23 00:11:15]

This constitutes work in a constructor which the styleguide disallows. :(

+  }
+
+  void CreateConsumer(Consumer** consumer, Producer* producer) {

[awong, 2011/08/23 00:11:15]

Overloaded functions are discouraged by the style guide. :(

How about CreateConsumerFromProducer and CreateConsumerFromConsumer?

+    WaitableEvent completion(true, false);
+    message_loop()->PostTask(
+        FROM_HERE,
+        NewRunnableFunction(&BackgroundThread::DoCreateFromProducer,
+                            consumer,
+                            producer,
+                            &completion));
+    completion.Wait();
+  }
+
+  void CreateConsumer(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 +248,91 @@ TEST(WeakPtrTest, SingleThreaded2) {
   EXPECT_EQ(&producer, consumer->producer.get());
 }
 
+TEST(WeakPtrTest, MoveOwnerShip) {

[awong, 2011/08/23 00:11:15]

OwnerShip -> Ownership

+  // Move object ownership to other thread by releasing all weak pointers
+  // on the original thread first.
+  // - 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;
+  Producer* producer = new Producer;

[awong, 2011/08/23 00:11:15]

new Producer();

+  {
+    WeakPtr<Producer> weak_ptr = producer->AsWeakPtr();
+  }
+  Consumer* consumer;
+  thread.CreateConsumer(&consumer, producer);
+  EXPECT_EQ(thread.DeRef(consumer), producer);
+  thread.DeleteProducer(producer);
+  thread.DeleteConsumer(consumer);
+}
+
+TEST(WeakPtrTest, DetachFromThread) {
+  // 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;
+  Producer producer;
+  Consumer* consumer;
+  thread.CreateConsumer(&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

[awong, 2011/08/23 00:11:15]

Capitalize "thread"

+  // - Destruct the pointer on thread A
+  BackgroundThread thread;
+  Producer producer;
+  Consumer consumer;
+  consumer.producer = producer.AsWeakPtr();
+  Consumer* consumer_copy;
+  thread.CreateConsumer(&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

[awong, 2011/08/23 00:11:15]

Capitalize "thread."

+  // - Destruct the pointer on thread B
+  BackgroundThread thread;
+  Producer producer;
+  Consumer* consumer_copy;
+  {
+    Consumer consumer;
+    consumer.producer = producer.AsWeakPtr();
+    thread.CreateConsumer(&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

[awong, 2011/08/23 00:11:15]

Capitalize "thread."

+  // - WeakPtr gets destroyed on thread B
+  BackgroundThread thread;
+  Consumer* consumer_copy;
+  {
+    Producer producer;
+    Consumer consumer;
+    consumer.producer = producer.AsWeakPtr();
+    thread.CreateConsumer(&consumer_copy, &consumer);
+  }
+  EXPECT_TRUE(consumer_copy->producer == NULL);
+  thread.DeleteConsumer(consumer_copy);
+}
+
 }  // namespace base