Allow linker initialization of lazy instance

base/lazy_instance.h

Index: base/lazy_instance.h
diff --git a/base/lazy_instance.h b/base/lazy_instance.h
index ac94a009111cf230ef70984add4b22034f53fdac..7812cf99b61e7e08dcdbee7850b08af2337a2dbe 100644
--- a/base/lazy_instance.h
+++ b/base/lazy_instance.h
@@ -24,7 +24,7 @@
 // requires that Type be a complete type so we can determine the size.
 //
 // Example usage:
-//   static LazyInstance<MyClass> my_instance(base::LINKER_INITIALIZED);
+//   static LazyInstance<MyClass> my_instance = LINKER_ZERO_INITIALIZED;

[Nico, 2011/11/11 20:21:56]

Is this needed? Aren't statics implicitly zero initialized? (See the definition
of LINKER_INITIALIZED for why that's there)

[joth, 2011/11/14 16:58:23]

Just needed as a double check to prove to the caller the instance really is POD
initializer-list initializable

 //   void SomeMethod() {
 //     my_instance.Get().SomeMethod();  // MyClass::SomeMethod()
 //
@@ -79,53 +79,44 @@ struct LeakyLazyInstanceTraits {
   }
 };
 
-// We pull out some of the functionality into a non-templated base, so that we
-// can implement the more complicated pieces out of line in the .cc file.
-class BASE_EXPORT LazyInstanceHelper {
- protected:
-  enum {
-    STATE_EMPTY    = 0,
-    STATE_CREATING = 1,
-    STATE_CREATED  = 2
-  };
+// We pull out some of the functionality into a non-templated functions, so we
+// we can implement the more complicated pieces out of line in the .cc file.
+namespace internal {
 
-  explicit LazyInstanceHelper(LinkerInitialized /*unused*/) {/* state_ is 0 */}
+// Our AtomicWord doubles as a spinlock, where a value of
+// kBeingCreatedMarker means the spinlock is being held for creation.
+static const subtle::AtomicWord kLazyInstanceStateCreating = 1;
 
-  // Declaring a destructor (even if it's empty) will cause MSVC to register a
-  // static initializer to register the empty destructor with atexit().
+// Declaring a destructor (even if it's empty) will cause MSVC to register a
+// static initializer to register the empty destructor with atexit().
 
-  // A destructor is intentionally not defined.  If we were to say
-  // ~LazyInstanceHelper() { }
-  // Even though it's empty, a destructor will still be generated.
-  // In order for the constructor to be called for static variables,
-  // it will be registered as a callback at runtime with AtExit().
-  // We don't want this, so we don't declare a destructor at all,
-  // effectively keeping the type POD (at least in terms of
-  // initialization and destruction).
-
-  // Check if instance needs to be created. If so return true otherwise
-  // if another thread has beat us, wait for instance to be created and
-  // return false.
-  bool NeedsInstance();
+// Check if instance needs to be created. If so return true otherwise
+// if another thread has beat us, wait for instance to be created and
+// return false.
+BASE_EXPORT bool NeedsLazyInstance(base::subtle::AtomicWord* state);
 
-  // After creating an instance, call this to register the dtor to be called
-  // at program exit and to update the state to STATE_CREATED.
-  void CompleteInstance(void* instance, void (*dtor)(void*));
+// After creating an instance, call this to register the dtor to be called
+// at program exit and to update the atomic state to hold the |new_instance|
+BASE_EXPORT void CompleteLazyInstance(base::subtle::AtomicWord* state,
+                                      base::subtle::AtomicWord new_instance,
+                                      void* lazy_instance,
+                                      void (*dtor)(void*));
 
-  base::subtle::Atomic32 state_;
+}  // namespace internal
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(LazyInstanceHelper);
-};
+#define LINKER_ZERO_INITIALIZED {0}
 
 template <typename Type, typename Traits = DefaultLazyInstanceTraits<Type> >
-class LazyInstance : public LazyInstanceHelper {
+class LazyInstance {
  public:
-  explicit LazyInstance(LinkerInitialized x) : LazyInstanceHelper(x) { }
-
   // Declaring a destructor (even if it's empty) will cause MSVC to register a
   // static initializer to register the empty destructor with atexit().
-  // Refer to the destructor-related comment in LazyInstanceHelper.
+  // Even though it's empty, a destructor will still be generated.
+  // In order for the constructor to be called for static variables,
+  // it will be registered as a callback at runtime with AtExit().
+  // We don't want this, so we don't declare a destructor at all,
+  // effectively keeping the type POD (at least in terms of
+  // initialization and destruction).
   // ~LazyInstance() {}
 
   Type& Get() {
@@ -140,57 +131,62 @@ class LazyInstance : public LazyInstanceHelper {
 #endif
 
     // We will hopefully have fast access when the instance is already created.
-    // Since a thread sees state_ != STATE_CREATED at most once,
+    // Since a thread sees private_instance_ == 0 or Creating at most once,

[willchan no longer on Chromium, 2011/11/11 22:18:21]

You mean kLazyInstanceStateCreating instead of just Creating, right?

[joth, 2011/11/14 16:58:23]

Done.

     // the load is taken out of NeedsInstance() as a fast-path.
     // The load has acquire memory ordering as a thread which sees
-    // state_ == STATE_CREATED needs to acquire visibility over
-    // the associated data (buf_). Pairing Release_Store is in
+    // private_instance_ > creating needs to acquire visibility over
+    // the associated data (private_buf_). Pairing Release_Store is in
     // CompleteInstance().
-    if ((base::subtle::Acquire_Load(&state_) != STATE_CREATED) &&
-        NeedsInstance()) {
-      // Create the instance in the space provided by |buf_|.
-      instance_ = Traits::New(buf_);
-      CompleteInstance(this, Traits::kRegisterOnExit ? OnExit : NULL);
+    base::subtle::AtomicWord value =

[willchan no longer on Chromium, 2011/11/11 22:18:21]

No need to use base:: from within the base namespace.

[joth, 2011/11/14 16:58:23]

Done. here & many other places in .h and .cc

+        base::subtle::Acquire_Load(&private_instance_);
+    if ((value == 0 || value == internal::kLazyInstanceStateCreating) &&

[willchan no longer on Chromium, 2011/11/11 22:18:21]

I'm not sure I understand why we changed away from using the state machine.
We're doing two compares here now (0 and kLazyInstanceStateCreating) rather than
a single != STATE_CREATED. Why? Are you making a space/time tradeoff here,
combining the state_ and instance_ variables into a single private_instance_
variable?

[joth, 2011/11/14 16:58:23]

Yep memory reduction plus consistency with Singleton.
but I remove the double comparison (by adding a bitwise and). 

+        internal::NeedsLazyInstance(&private_instance_)) {
+      // Create the instance in the space provided by |private_buf_|.
+      value = reinterpret_cast<base::subtle::AtomicWord>(
+          Traits::New(private_buf_));
+      internal::CompleteLazyInstance(&private_instance_, value, this,
+                                     Traits::kRegisterOnExit ? OnExit : NULL);
     }
 
     // This annotation helps race detectors recognize correct lock-less
     // synchronization between different threads calling Pointer().
     // We suggest dynamic race detection tool that "Traits::New" above
-    // and CompleteInstance(...) happens before "return instance_" below.
+    // and CompleteInstance(...) happens before "return instance()" below.
     // See the corresponding HAPPENS_BEFORE in CompleteInstance(...).
-    ANNOTATE_HAPPENS_AFTER(&state_);
-    return instance_;
+    ANNOTATE_HAPPENS_AFTER(&private_instance_);
+    return instance();
   }
 
   bool operator==(Type* p) {
-    switch (base::subtle::NoBarrier_Load(&state_)) {
-      case STATE_EMPTY:
+    switch (base::subtle::NoBarrier_Load(&private_instance_)) {
+      case 0:
         return p == NULL;
-      case STATE_CREATING:
-        return static_cast<int8*>(static_cast<void*>(p)) == buf_;
-      case STATE_CREATED:
-        return p == instance_;
+      case internal::kLazyInstanceStateCreating:
+        return static_cast<int8*>(static_cast<void*>(p)) == private_buf_;
       default:
-        return false;
+        return p == instance();
     }
   }
 
+  // Effectively private: member data is only public to allow the linker to

[Nico, 2011/11/11 20:21:56]

Sentence cut off

[joth, 2011/11/11 21:15:36]

Done.

+  // Note this must use AtomicWord, not Atomic32, to ensure correct alignment of
+  // |private_buf_| on 64 bit arcchitectures.

[Nico, 2011/11/11 20:21:56]

spello arcchitectures

[joth, 2011/11/11 21:15:36]

Done.

+  base::subtle::AtomicWord private_instance_;
+  // statically initialize it. DO NOT USE FROM OUTSIDE THIS CLASS.
+  int8 private_buf_[sizeof(Type)];  // Preallocated space for the Type instance.
+
  private:
+  Type* instance() { return reinterpret_cast<Type*>(private_instance_); }
+
   // Adapter function for use with AtExit.  This should be called single
   // threaded, so don't synchronize across threads.
   // Calling OnExit while the instance is in use by other threads is a mistake.
   static void OnExit(void* lazy_instance) {
     LazyInstance<Type, Traits>* me =
         reinterpret_cast<LazyInstance<Type, Traits>*>(lazy_instance);
-    Traits::Delete(me->instance_);
-    me->instance_ = NULL;
-    base::subtle::Release_Store(&me->state_, STATE_EMPTY);
+    Traits::Delete(me->instance());
+    base::subtle::Release_Store(&me->private_instance_, 0);
   }
-
-  Type *instance_;
-  int8 buf_[sizeof(Type)];  // Preallocate the space for the Type instance.
-
-  DISALLOW_COPY_AND_ASSIGN(LazyInstance);
 };
 
 }  // namespace base