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

base/memory/weak_ptr.h

Index: base/memory/weak_ptr.h
diff --git a/base/memory/weak_ptr.h b/base/memory/weak_ptr.h
index abda1af2eaa7e0abf54a9a79bd0d9f7a4a526538..9a39696f2c44e1f8e2ace42927384ab6fffe1cd9 100644
--- a/base/memory/weak_ptr.h
+++ b/base/memory/weak_ptr.h
@@ -45,8 +45,18 @@
 // 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.
+// Thread-safety notes: Weak pointers may only be dereferenced on the thread

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

This passage is a bit complicated.

I think you might want to rework it like so:

  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.

That last bit may be better placed above the HasWeakPtrs() definition.  I also
don't think you need to document HasRefs() since that is not part of the public
API.

+// on which the object being pointed to will be deleted. However, WeakPtr
+// references themselves may be be passed, copied and destroyed across threads.
+// Whenever there are no weak references, the thread on which the first
+// reference is created initializes the thread-safety checks with respect
+// to querying and invalidating the reference.
+// Getting the initial reference and querying HasWeakPtrs()/HasRefs() also
+// should only be done on the owning thread and the user has to avoid races.
+// Again, note that you can however create an object on one thread, and
+// then transfer ownership to another thread on which it will be deleted.
+// When transferring ownership, all previous weak references must first be
+// released.
 
 #ifndef BASE_MEMORY_WEAK_PTR_H_
 #define BASE_MEMORY_WEAK_PTR_H_
@@ -69,7 +79,7 @@ class BASE_EXPORT WeakReference {
   // via base::WeakPtr::~WeakPtr().
   class Flag : public RefCountedThreadSafe<Flag> {
    public:
-    explicit Flag(Flag** handle);
+    explicit Flag();
 
     void Invalidate();
     bool IsValid() const;
@@ -82,17 +92,17 @@ class BASE_EXPORT WeakReference {
     ~Flag();
 
     ThreadChecker thread_checker_;
-    Flag** handle_;
+    bool is_valid_;
   };
 
   WeakReference();
-  WeakReference(Flag* flag);
+  WeakReference(const Flag* flag);
   ~WeakReference();
 
   bool is_valid() const;
 
  private:
-  scoped_refptr<Flag> flag_;
+  scoped_refptr<const Flag> flag_;
 };
 
 class BASE_EXPORT WeakReferenceOwner {
@@ -103,7 +113,7 @@ class BASE_EXPORT WeakReferenceOwner {
   WeakReference GetRef() const;
 
   bool HasRefs() const {
-    return flag_ != NULL;
+    return flag_.get() && !flag_->HasOneRef();
   }
 
   void Invalidate();
@@ -114,7 +124,7 @@ class BASE_EXPORT WeakReferenceOwner {
   }
 
  private:
-  mutable WeakReference::Flag* flag_;
+  mutable scoped_refptr<WeakReference::Flag> flag_;
 };
 
 // This class simplifies the implementation of WeakPtr's type conversion