Revert 66719 - Reland r65996. Disallows Singletons on non-joinable thread.

Test breakages caused by this change have been fixed here or in other changelists.

BUG=61753
TEST=none

Review URL: http://codereview.chromium.org/5024003

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/5206005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@66791 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 41 insertions, 180 deletions

diff --git a/base/lazy_instance.h b/base/lazy_instance.h
index f8d5987..ebf1e73 100644
--- a/base/lazy_instance.h
+++ b/base/lazy_instance.h
@@ -36,19 +36,14 @@
 #define BASE_LAZY_INSTANCE_H_
 #pragma once
 
-#include <new>  // For placement new.
-
 #include "base/atomicops.h"
 #include "base/basictypes.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
-#include "base/thread_restrictions.h"
 
 namespace base {
 
 template <typename Type>
 struct DefaultLazyInstanceTraits {
-  static const bool kAllowedToAccessOnNonjoinableThread = false;
-
   static Type* New(void* instance) {
     // Use placement new to initialize our instance in our preallocated space.
     // The parenthesis is very important here to force POD type initialization.
@@ -62,8 +57,6 @@ struct DefaultLazyInstanceTraits {
 
 template <typename Type>
 struct LeakyLazyInstanceTraits {
-  static const bool kAllowedToAccessOnNonjoinableThread = true;
-
   static Type* New(void* instance) {
     return DefaultLazyInstanceTraits<Type>::New(instance);
   }
@@ -119,9 +112,6 @@ class LazyInstance : public LazyInstanceHelper {
   }
 
   Type* Pointer() {
-    if (!Traits::kAllowedToAccessOnNonjoinableThread)
-      base::ThreadRestrictions::AssertSingletonAllowed();
-
     // We will hopefully have fast access when the instance is already created.
     if ((base::subtle::NoBarrier_Load(&state_) != STATE_CREATED) &&
         NeedsInstance()) {
@@ -136,6 +126,7 @@ class LazyInstance : public LazyInstanceHelper {
     // and CompleteInstance(...) happens before "return instance_" below.
     // See the corresponding HAPPENS_BEFORE in CompleteInstance(...).
     ANNOTATE_HAPPENS_AFTER(&state_);
+
     return instance_;
   }
 
diff --git a/base/message_loop_proxy_impl.cc b/base/message_loop_proxy_impl.cc
index 7998a94..2e0f809 100644
--- a/base/message_loop_proxy_impl.cc
+++ b/base/message_loop_proxy_impl.cc
@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "base/message_loop_proxy_impl.h"
-#include "base/thread_restrictions.h"
 
 namespace base {
 
@@ -46,11 +45,6 @@ bool MessageLoopProxyImpl::PostNonNestableDelayedTask(
 }
 
 bool MessageLoopProxyImpl::BelongsToCurrentThread() {
-  // We shouldn't use MessageLoop::current() since it uses LazyInstance which
-  // may be deleted by ~AtExitManager when a WorkerPool thread calls this
-  // function.
-  // http://crbug.com/63678
-  base::ThreadRestrictions::ScopedAllowSingleton allow_singleton;
   AutoLock lock(message_loop_lock_);
   return (target_message_loop_ &&
           (MessageLoop::current() == target_message_loop_));
@@ -104,3 +98,4 @@ MessageLoopProxy::CreateForCurrentThread() {
 }
 
 }  // namespace base
+
diff --git a/base/platform_thread_posix.cc b/base/platform_thread_posix.cc
index e442e9c..66f3928 100644
--- a/base/platform_thread_posix.cc
+++ b/base/platform_thread_posix.cc
@@ -7,11 +7,6 @@
 #include <errno.h>
 #include <sched.h>
 
-#include "base/logging.h"
-#include "base/safe_strerror_posix.h"
-#include "base/scoped_ptr.h"
-#include "base/thread_restrictions.h"
-
 #if defined(OS_MACOSX)
 #include <mach/mach.h>
 #include <sys/resource.h>
@@ -29,27 +24,18 @@
 #include <sys/nacl_syscalls.h>
 #endif
 
+#include "base/logging.h"
+#include "base/safe_strerror_posix.h"
+
 #if defined(OS_MACOSX)
 namespace base {
 void InitThreading();
 }  // namespace base
 #endif
 
-namespace {
-
-struct ThreadParams {
-  PlatformThread::Delegate* delegate;
-  bool joinable;
-};
-
-}  // namespace
-
-static void* ThreadFunc(void* params) {
-  ThreadParams* thread_params = static_cast<ThreadParams*>(params);
-  PlatformThread::Delegate* delegate = thread_params->delegate;
-  if (!thread_params->joinable)
-    base::ThreadRestrictions::SetSingletonAllowed(false);
-  delete thread_params;
+static void* ThreadFunc(void* closure) {
+  PlatformThread::Delegate* delegate =
+      static_cast<PlatformThread::Delegate*>(closure);
   delegate->ThreadMain();
   return NULL;
 }
@@ -188,14 +174,9 @@ bool CreateThread(size_t stack_size, bool joinable,
   if (stack_size > 0)
     pthread_attr_setstacksize(&attributes, stack_size);
 
-  ThreadParams* params = new ThreadParams;
-  params->delegate = delegate;
-  params->joinable = joinable;
-  success = !pthread_create(thread_handle, &attributes, ThreadFunc, params);
+  success = !pthread_create(thread_handle, &attributes, ThreadFunc, delegate);
 
   pthread_attr_destroy(&attributes);
-  if (!success)
-    delete params;
   return success;
 }
 
diff --git a/base/platform_thread_win.cc b/base/platform_thread_win.cc
index e5afc52..a4a1b52 100644
--- a/base/platform_thread_win.cc
+++ b/base/platform_thread_win.cc
@@ -5,7 +5,6 @@
 #include "base/platform_thread.h"
 
 #include "base/logging.h"
-#include "base/thread_restrictions.h"
 #include "base/win/windows_version.h"
 
 namespace {
@@ -21,58 +20,13 @@ typedef struct tagTHREADNAME_INFO {
   DWORD dwFlags;  // Reserved for future use, must be zero.
 } THREADNAME_INFO;
 
-struct ThreadParams {
-  PlatformThread::Delegate* delegate;
-  bool joinable;
-};
-
-DWORD __stdcall ThreadFunc(void* params) {
-  ThreadParams* thread_params = static_cast<ThreadParams*>(params);
-  PlatformThread::Delegate* delegate = thread_params->delegate;
-  if (!thread_params->joinable)
-    base::ThreadRestrictions::SetSingletonAllowed(false);
-  delete thread_params;
+DWORD __stdcall ThreadFunc(void* closure) {
+  PlatformThread::Delegate* delegate =
+      static_cast<PlatformThread::Delegate*>(closure);
   delegate->ThreadMain();
   return NULL;
 }
 
-// CreateThreadInternal() matches PlatformThread::Create(), except that
-// |out_thread_handle| may be NULL, in which case a non-joinable thread is
-// created.
-bool CreateThreadInternal(size_t stack_size,
-                          PlatformThread::Delegate* delegate,
-                          PlatformThreadHandle* out_thread_handle) {
-  PlatformThreadHandle thread_handle;
-  unsigned int flags = 0;
-  if (stack_size > 0 && base::win::GetVersion() >= base::win::VERSION_XP) {
-    flags = STACK_SIZE_PARAM_IS_A_RESERVATION;
-  } else {
-    stack_size = 0;
-  }
-
-  ThreadParams* params = new ThreadParams;
-  params->delegate = delegate;
-  params->joinable = out_thread_handle != NULL;
-
-  // Using CreateThread here vs _beginthreadex makes thread creation a bit
-  // faster and doesn't require the loader lock to be available.  Our code will
-  // have to work running on CreateThread() threads anyway, since we run code
-  // on the Windows thread pool, etc.  For some background on the difference:
-  //   http://www.microsoft.com/msj/1099/win32/win321099.aspx
-  thread_handle = CreateThread(
-      NULL, stack_size, ThreadFunc, params, flags, NULL);
-  if (!thread_handle) {
-    delete params;
-    return false;
-  }
-
-  if (out_thread_handle)
-    *out_thread_handle = thread_handle;
-  else
-    CloseHandle(thread_handle);
-  return true;
-}
-
 }  // namespace
 
 // static
@@ -113,13 +67,29 @@ void PlatformThread::SetName(const char* name) {
 // static
 bool PlatformThread::Create(size_t stack_size, Delegate* delegate,
                             PlatformThreadHandle* thread_handle) {
-  DCHECK(thread_handle);
-  return CreateThreadInternal(stack_size, delegate, thread_handle);
+  unsigned int flags = 0;
+  if (stack_size > 0 && base::win::GetVersion() >= base::win::VERSION_XP) {
+    flags = STACK_SIZE_PARAM_IS_A_RESERVATION;
+  } else {
+    stack_size = 0;
+  }
+
+  // Using CreateThread here vs _beginthreadex makes thread creation a bit
+  // faster and doesn't require the loader lock to be available.  Our code will
+  // have to work running on CreateThread() threads anyway, since we run code
+  // on the Windows thread pool, etc.  For some background on the difference:
+  //   http://www.microsoft.com/msj/1099/win32/win321099.aspx
+  *thread_handle = CreateThread(
+      NULL, stack_size, ThreadFunc, delegate, flags, NULL);
+  return *thread_handle != NULL;
 }
 
 // static
 bool PlatformThread::CreateNonJoinable(size_t stack_size, Delegate* delegate) {
-  return CreateThreadInternal(stack_size, delegate, NULL);
+  PlatformThreadHandle thread_handle;
+  bool result = Create(stack_size, delegate, &thread_handle);
+  CloseHandle(thread_handle);
+  return result;
 }
 
 // static
diff --git a/base/singleton.h b/base/singleton.h
index 7caa8ee..564b686 100644
--- a/base/singleton.h
+++ b/base/singleton.h
@@ -9,7 +9,6 @@
 #include "base/at_exit.h"
 #include "base/atomicops.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
-#include "base/thread_restrictions.h"
 
 // Default traits for Singleton<Type>. Calls operator new and operator delete on
 // the object. Registers automatic deletion at process exit.
@@ -31,11 +30,6 @@ struct DefaultSingletonTraits {
   // Set to true to automatically register deletion of the object on process
   // exit. See below for the required call that makes this happen.
   static const bool kRegisterAtExit = true;
-
-  // Set to false to disallow access on a non-joinable thread.  This is
-  // different from kRegisterAtExit because StaticMemorySingletonTraits allows
-  // access on non-joinable threads, and gracefully handles this.
-  static const bool kAllowedToAccessOnNonjoinableThread = false;
 };
 
 
@@ -45,7 +39,6 @@ struct DefaultSingletonTraits {
 template<typename Type>
 struct LeakySingletonTraits : public DefaultSingletonTraits<Type> {
   static const bool kRegisterAtExit = false;
-  static const bool kAllowedToAccessOnNonjoinableThread = true;
 };
 
 
@@ -92,7 +85,6 @@ struct StaticMemorySingletonTraits {
   }
 
   static const bool kRegisterAtExit = true;
-  static const bool kAllowedToAccessOnNonjoinableThread = true;
 
   // Exposed for unittesting.
   static void Resurrect() {
@@ -184,9 +176,6 @@ class Singleton {
 
   // Return a pointer to the one true instance of the class.
   static Type* get() {
-    if (!Traits::kAllowedToAccessOnNonjoinableThread)
-      base::ThreadRestrictions::AssertSingletonAllowed();
-
     // Our AtomicWord doubles as a spinlock, where a value of
     // kBeingCreatedMarker means the spinlock is being held for creation.
     static const base::subtle::AtomicWord kBeingCreatedMarker = 1;
diff --git a/base/thread_restrictions.cc b/base/thread_restrictions.cc
index 6767d80..270d663 100644
--- a/base/thread_restrictions.cc
+++ b/base/thread_restrictions.cc
@@ -18,16 +18,13 @@ namespace {
 LazyInstance<ThreadLocalBoolean, LeakyLazyInstanceTraits<ThreadLocalBoolean> >
     g_io_disallowed(LINKER_INITIALIZED);
 
-LazyInstance<ThreadLocalBoolean, LeakyLazyInstanceTraits<ThreadLocalBoolean> >
-    g_singleton_disallowed(LINKER_INITIALIZED);
-
 }  // anonymous namespace
 
 // static
 bool ThreadRestrictions::SetIOAllowed(bool allowed) {
-  bool previous_disallowed = g_io_disallowed.Get().Get();
+  bool previous_allowed = g_io_disallowed.Get().Get();
   g_io_disallowed.Get().Set(!allowed);
-  return !previous_disallowed;
+  return !previous_allowed;
 }
 
 // static
@@ -42,22 +39,6 @@ void ThreadRestrictions::AssertIOAllowed() {
   }
 }
 
-bool ThreadRestrictions::SetSingletonAllowed(bool allowed) {
-  bool previous_disallowed = g_singleton_disallowed.Get().Get();
-  g_singleton_disallowed.Get().Set(!allowed);
-  return !previous_disallowed;
-}
-
-// static
-void ThreadRestrictions::AssertSingletonAllowed() {
-  if (g_singleton_disallowed.Get().Get()) {
-    LOG(FATAL) << "LazyInstance/Singleton is not allowed to be used on this "
-               << "thread.  Most likely it's because this thread is not "
-               << "joinable, so AtExitManager may have deleted the object "
-               << "on shutdown, leading to a potential shutdown crash.";
-  }
-}
-
 }  // namespace base
 
 #endif  // NDEBUG
diff --git a/base/thread_restrictions.h b/base/thread_restrictions.h
index 51e5a15..a10aaec 100644
--- a/base/thread_restrictions.h
+++ b/base/thread_restrictions.h
@@ -9,13 +9,8 @@
 
 namespace base {
 
-// Certain behavior is disallowed on certain threads.  ThreadRestrictions helps
-// enforce these rules.  Examples of such rules:
-//
-// * Do not do blocking IO (makes the thread janky)
-// * Do not access Singleton/LazyInstance (may lead to shutdown crashes)
-//
-// Here's more about how the protection works:
+// ThreadRestrictions helps protect threads that should not block from
+// making blocking calls.  It works like this:
 //
 // 1) If a thread should not be allowed to make IO calls, mark it:
 //      base::ThreadRestrictions::SetIOAllowed(false);
@@ -50,20 +45,6 @@ class ThreadRestrictions {
     DISALLOW_COPY_AND_ASSIGN(ScopedAllowIO);
   };
 
-  // Constructing a ScopedAllowSingleton temporarily allows accessing for the
-  // current thread.  Doing this is almost always incorrect.
-  class ScopedAllowSingleton {
-   public:
-    ScopedAllowSingleton() { previous_value_ = SetSingletonAllowed(true); }
-    ~ScopedAllowSingleton() { SetSingletonAllowed(previous_value_); }
-   private:
-    // Whether singleton use is allowed when the ScopedAllowSingleton was
-    // constructed.
-    bool previous_value_;
-
-    DISALLOW_COPY_AND_ASSIGN(ScopedAllowSingleton);
-  };
-
 #ifndef NDEBUG
   // Set whether the current thread to make IO calls.
   // Threads start out in the *allowed* state.
@@ -74,25 +55,15 @@ class ThreadRestrictions {
   // and DCHECK if not.  See the block comment above the class for
   // a discussion of where to add these checks.
   static void AssertIOAllowed();
-
-  // Set whether the current thread can use singletons.  Returns the previous
-  // value.
-  static bool SetSingletonAllowed(bool allowed);
-
-  // Check whether the current thread is allowed to use singletons (Singleton /
-  // LazyInstance).  DCHECKs if not.
-  static void AssertSingletonAllowed();
 #else
   // In Release builds, inline the empty definitions of these functions so
   // that they can be compiled out.
   static bool SetIOAllowed(bool allowed) { return true; }
   static void AssertIOAllowed() {}
-  static bool SetSingletonAllowed(bool allowed) { return true; }
-  static void AssertSingletonAllowed() {}
 #endif
 
  private:
-  DISALLOW_IMPLICIT_CONSTRUCTORS(ThreadRestrictions);
+  ThreadRestrictions();  // class for namespacing only
 };
 
 }  // namespace base
diff --git a/base/tracked_objects.cc b/base/tracked_objects.cc
index 9db25ff..4631f34 100644
--- a/base/tracked_objects.cc
+++ b/base/tracked_objects.cc
@@ -10,7 +10,6 @@
 #include "base/message_loop.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
-#include "base/thread_restrictions.h"
 
 using base::TimeDelta;
 
@@ -90,13 +89,7 @@ Lock ThreadData::list_lock_;
 // static
 ThreadData::Status ThreadData::status_ = ThreadData::UNINITIALIZED;
 
-ThreadData::ThreadData() : next_(NULL) {
-  // This shouldn't use the MessageLoop::current() LazyInstance since this might
-  // be used on a non-joinable thread.
-  // http://crbug.com/62728
-  base::ThreadRestrictions::ScopedAllowSingleton scoped_allow_singleton;
-  message_loop_ = MessageLoop::current();
-}
+ThreadData::ThreadData() : next_(NULL), message_loop_(MessageLoop::current()) {}
 
 ThreadData::~ThreadData() {}
 
@@ -267,14 +260,8 @@ void ThreadData::WriteHTMLTotalAndSubtotals(
 }
 
 Births* ThreadData::TallyABirth(const Location& location) {
-  {
-    // This shouldn't use the MessageLoop::current() LazyInstance since this
-    // might be used on a non-joinable thread.
-    // http://crbug.com/62728
-    base::ThreadRestrictions::ScopedAllowSingleton scoped_allow_singleton;
-    if (!message_loop_)  // In case message loop wasn't yet around...
-      message_loop_ = MessageLoop::current();  // Find it now.
-  }
+  if (!message_loop_)  // In case message loop wasn't yet around...
+    message_loop_ = MessageLoop::current();  // Find it now.
 
   BirthMap::iterator it = birth_map_.find(location);
   if (it != birth_map_.end()) {
@@ -292,12 +279,8 @@ Births* ThreadData::TallyABirth(const Location& location) {
 
 void ThreadData::TallyADeath(const Births& lifetimes,
                              const TimeDelta& duration) {
-  {
-    // http://crbug.com/62728
-    base::ThreadRestrictions::ScopedAllowSingleton scoped_allow_singleton;
-    if (!message_loop_)  // In case message loop wasn't yet around...
-      message_loop_ = MessageLoop::current();  // Find it now.
-  }
+  if (!message_loop_)  // In case message loop wasn't yet around...
+    message_loop_ = MessageLoop::current();  // Find it now.
 
   DeathMap::iterator it = death_map_.find(&lifetimes);
   if (it != death_map_.end()) {