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

base/memory/weak_ptr.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Weak pointers help in cases where you have many objects referring back to a
 // shared object and you wish for the lifetime of the shared object to not be
 // bound to the lifetime of the referrers.  In other words, this is useful when
 // reference counting is not a good fit.
 //
 // A common alternative to weak pointers is to have the shared object hold a
@@ skipping 27 matching lines @@
 //    WeakPtr<Controller> controller_;
 //  };
 //
 // Given the above classes, a consumer may allocate a Controller object, call
 // SpawnWorker several times, and then destroy the Controller object before all
 // of the workers have completed.  Because the Worker class only holds a weak
 // pointer to the Controller, we don't have to worry about the Worker
 // 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_
 #pragma once
 
 #include "base/base_export.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/threading/thread_checker.h"
 
 namespace base {
 
 namespace internal {
 // These classes are part of the WeakPtr implementation.
 // DO NOT USE THESE CLASSES DIRECTLY YOURSELF.
 
 class BASE_EXPORT WeakReference {
  public:
   // While Flag is bound to a specific thread, it may be deleted from another
   // via base::WeakPtr::~WeakPtr().
   class Flag : public RefCountedThreadSafe<Flag> {
    public:
-    explicit Flag(Flag** handle);
+    explicit Flag();
 
     void Invalidate();
     bool IsValid() const;
 
     void DetachFromThread() { thread_checker_.DetachFromThread(); }
 
    private:
     friend class base::RefCountedThreadSafe<Flag>;
 
     ~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 {
  public:
   WeakReferenceOwner();
   ~WeakReferenceOwner();
 
   WeakReference GetRef() const;
 
   bool HasRefs() const {
-    return flag_ != NULL;
+    return flag_.get() && !flag_->HasOneRef();
   }
 
   void Invalidate();
 
   // Indicates that this object will be used on another thread from now on.
   void DetachFromThread() {
     if (flag_) flag_->DetachFromThread();
   }
 
  private:
-  mutable WeakReference::Flag* flag_;
+  mutable scoped_refptr<WeakReference::Flag> flag_;
 };
 
 // This class simplifies the implementation of WeakPtr's type conversion
 // constructor by avoiding the need for a public accessor for ref_.  A
 // WeakPtr<T> cannot access the private members of WeakPtr<U>, so this
 // base class gives us a way to access ref_ in a protected fashion.
 class BASE_EXPORT WeakPtrBase {
  public:
   WeakPtrBase();
   ~WeakPtrBase();
@@ skipping 118 matching lines @@
 
  private:
   internal::WeakReferenceOwner weak_reference_owner_;
   T* ptr_;
   DISALLOW_IMPLICIT_CONSTRUCTORS(WeakPtrFactory);
 };
 
 }  // namespace base
 
 #endif  // BASE_MEMORY_WEAK_PTR_H_