2 files changed, 39 insertions, 9 deletions

diff --git a/base/lazy_instance.cc b/base/lazy_instance.cc
index 32bb788..1be3488 100644
--- a/base/lazy_instance.cc
+++ b/base/lazy_instance.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -15,15 +15,21 @@ namespace base {
 bool LazyInstanceHelper::NeedsInstance() {
   // Try to create the instance, if we're the first, will go from EMPTY
   // to CREATING, otherwise we've already been beaten here.
-  if (base::subtle::Acquire_CompareAndSwap(
-          &state_, STATE_EMPTY, STATE_CREATING) == STATE_EMPTY) {
+  // The memory access has no memory ordering as STATE_EMPTY and STATE_CREATING
+  // has no associated data (memory barriers are all about ordering
+  // of memory accesses to *associated* data).
+  if (base::subtle::NoBarrier_CompareAndSwap(
+          &state_, STATE_EMPTY, STATE_CREATING) == STATE_EMPTY)
     // Caller must create instance
     return true;
-  } else {
-    // It's either in the process of being created, or already created.  Spin.
-    while (base::subtle::NoBarrier_Load(&state_) != STATE_CREATED)
-      PlatformThread::YieldCurrentThread();
-  }
+
+  // It's either in the process of being created, or already created. Spin.
+  // 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
+  // CompleteInstance().
+  while (base::subtle::Acquire_Load(&state_) != STATE_CREATED)
+    PlatformThread::YieldCurrentThread();
 
   // Someone else created the instance.
   return false;
@@ -34,6 +40,8 @@ void LazyInstanceHelper::CompleteInstance(void* instance, void (*dtor)(void*)) {
   ANNOTATE_HAPPENS_BEFORE(&state_);
 
   // Instance is created, go from CREATING to CREATED.
+  // Releases visibility over buf_ to readers. Pairing Acquire_Load's are in
+  // NeedsInstance() and Pointer().
   base::subtle::Release_Store(&state_, STATE_CREATED);
 
   // Make sure that the lazily instantiated object will get destroyed at exit.
@@ -42,3 +50,6 @@ void LazyInstanceHelper::CompleteInstance(void* instance, void (*dtor)(void*)) {
 }
 
 }  // namespace base
+
+
+
diff --git a/base/lazy_instance.h b/base/lazy_instance.h
index 7b1bdc4..49c7b8c 100644
--- a/base/lazy_instance.h
+++ b/base/lazy_instance.h
@@ -90,9 +90,19 @@ class BASE_API LazyInstanceHelper {
   };
 
   explicit LazyInstanceHelper(LinkerInitialized /*unused*/) {/* state_ is 0 */}
+
   // 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.
@@ -112,8 +122,11 @@ template <typename Type, typename Traits = DefaultLazyInstanceTraits<Type> >
 class LazyInstance : public LazyInstanceHelper {
  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.
+  // ~LazyInstance() {}
 
   Type& Get() {
     return *Pointer();
@@ -124,7 +137,13 @@ class LazyInstance : public LazyInstanceHelper {
       base::ThreadRestrictions::AssertSingletonAllowed();
 
     // We will hopefully have fast access when the instance is already created.
-    if ((base::subtle::NoBarrier_Load(&state_) != STATE_CREATED) &&
+    // Since a thread sees state_ != STATE_CREATED at most once,
+    // 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
+    // CompleteInstance().
+    if ((base::subtle::Acquire_Load(&state_) != STATE_CREATED) &&
         NeedsInstance()) {
       // Create the instance in the space provided by |buf_|.
       instance_ = Traits::New(buf_);