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

base/memory/weak_ptr.cc

Index: base/memory/weak_ptr.cc
diff --git a/base/memory/weak_ptr.cc b/base/memory/weak_ptr.cc
index 30c777c00cf1b373e17b6c811e9aaf71fab86f11..bd2fb6497f04fe23d5440eaf432a672cee99a82d 100644
--- a/base/memory/weak_ptr.cc
+++ b/base/memory/weak_ptr.cc
@@ -7,28 +7,26 @@
 namespace base {
 namespace internal {
 
-WeakReference::Flag::Flag(Flag** handle) : handle_(handle) {
+WeakReference::Flag::Flag() : valid_(true) {
 }
 
 void WeakReference::Flag::Invalidate() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  handle_ = NULL;
+  valid_ = false;
 }
 
 bool WeakReference::Flag::IsValid() const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  return handle_ != NULL;
+  return valid_;
 }
 
 WeakReference::Flag::~Flag() {
-  if (handle_)
-    *handle_ = NULL;
 }
 
 WeakReference::WeakReference() {
 }
 
-WeakReference::WeakReference(Flag* flag) : flag_(flag) {
+WeakReference::WeakReference(scoped_refptr<Flag> flag) : flag_(flag) {
 }
 
 WeakReference::~WeakReference() {
@@ -46,8 +44,10 @@ WeakReferenceOwner::~WeakReferenceOwner() {
 }
 
 WeakReference WeakReferenceOwner::GetRef() const {
-  if (!flag_)
-    flag_ = new WeakReference::Flag(&flag_);
+  // We also want to reattach to the current thread if all previous references
+  // have gone away.
+  if (!flag_ || flag_->HasOneRef())

[darin (slow to review), 2011/08/18 18:29:06]

perhaps it would be better to reuse the HasRefs() implementation?

  if (!HasRefs())
    flag_ = new WeakReference::Flag();

[willchan no longer on Chromium, 2011/08/18 18:49:52]

Sorry, it's not clear to me how this is safe.

If base::WeakPtr is destroyed on another thread, then its WeakReference will go
away on that other thread, right? That means the refcount for
WeakReference::Flag may be decremented on another thread (or incremented if the
WeakPtr is copied on another thread). So I don't understand why an
implementation that uses HasOneRef() where refcounts are modified in multiple
threads can be safe. Is this race benign somehow? Sorry, didn't sleep on the
plane and stuck at a transfer in Dubai. Comments here explaining the
implementation detail would be nice.

+    flag_ = new WeakReference::Flag();
   return WeakReference(flag_);
 }