Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1646)

Unified Diff: base/at_exit.cc

Issue 1661673003: Fix AtExitManager recursive lock acquisition. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Update comment. Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/at_exit.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/at_exit.cc
diff --git a/base/at_exit.cc b/base/at_exit.cc
index 0fba355698fbebc643fd3225d1323ec64964eb7d..f9aa2d1094e9c65aaf60df9c7681c606c7ec4e27 100644
--- a/base/at_exit.cc
+++ b/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;
}
« no previous file with comments | « base/at_exit.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698