Fix AtExitManager recursive lock acquisition.

Also detect when new callbacks are being added while callbacks are being processed.

Review URL: https://codereview.chromium.org/1661673003

Cr-Commit-Position: refs/heads/master@{#374575}

base/at_exit.cc

@@ -21,7 +21,8 @@ namespace base {
 // this for thread-safe access, since it will only be modified in testing.
 static AtExitManager* g_top_manager = NULL;
 
-AtExitManager::AtExitManager() : next_manager_(g_top_manager) {
+AtExitManager::AtExitManager()
+    : processing_callbacks_(false), next_manager_(g_top_manager) {
 // If multiple modules instantiate AtExitManagers they'll end up living in this
 // module... they have to coexist.
 #if !defined(COMPONENT_BUILD)
@@ -55,6 +56,7 @@ void AtExitManager::RegisterTask(base::Closure task) {
   }
 
   AutoLock lock(g_top_manager->lock_);
+  DCHECK(!g_top_manager->processing_callbacks_);
   g_top_manager->stack_.push(task);
 }
 
@@ -65,16 +67,28 @@ void AtExitManager::ProcessCallbacksNow() {
     return;
   }
 
-  AutoLock lock(g_top_manager->lock_);
+  // Callbacks may try to add new callbacks, so run them without holding
+  // |lock_|. This is an error and caught by the DCHECK in RegisterTask(), but
+  // handle it gracefully in release builds so we don't deadlock.
+  std::stack<base::Closure> tasks;
+  {
+    AutoLock lock(g_top_manager->lock_);
+    tasks.swap(g_top_manager->stack_);
+    g_top_manager->processing_callbacks_ = true;
+  }
 
-  while (!g_top_manager->stack_.empty()) {
-    base::Closure task = g_top_manager->stack_.top();
+  while (!tasks.empty()) {
+    base::Closure task = tasks.top();
     task.Run();
-    g_top_manager->stack_.pop();
+    tasks.pop();
   }
+
+  // Expect that all callbacks have been run.
+  DCHECK(g_top_manager->stack_.empty());
 }
 
-AtExitManager::AtExitManager(bool shadow) : next_manager_(g_top_manager) {
+AtExitManager::AtExitManager(bool shadow)
+    : processing_callbacks_(false), next_manager_(g_top_manager) {
   DCHECK(shadow || !g_top_manager);
   g_top_manager = this;
 }

base/at_exit.h

@@ -59,6 +59,7 @@ class BASE_EXPORT AtExitManager {
  private:
   base::Lock lock_;
   std::stack<base::Closure> stack_;
+  bool processing_callbacks_;
   AtExitManager* next_manager_;  // Stack of managers to allow shadowing.
 
   DISALLOW_COPY_AND_ASSIGN(AtExitManager);