Making the MemoryDumpManager less aggressive in disabling dump providers on failures.

base/trace_event/memory_dump_manager.cc

 // Copyright 2015 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.
 
 #include "base/trace_event/memory_dump_manager.h"
 
 #include <algorithm>
 
 #include "base/atomic_sequence_num.h"
 #include "base/compiler_specific.h"
@@ skipping 299 matching lines @@
   // straight away (FinalizeDumpAndAddToTrace() takes care of the callback).
   if (did_any_provider_dump && pmd_holder->num_pending_async_requests == 0)
     FinalizeDumpAndAddToTrace(pmd_holder);
 }
 
 // Invokes the MemoryDumpProvider.OnMemoryDump(), taking care of the fail-safe
 // logic which disables the dumper when failing (crbug.com/461788).
 bool MemoryDumpManager::InvokeDumpProviderLocked(MemoryDumpProvider* mdp,
                                                  ProcessMemoryDump* pmd) {
   lock_.AssertAcquired();
+  const int kMaxConsecutiveFailuresCount = 3;
   bool dump_successful = mdp->OnMemoryDump(pmd);
+  MemoryDumpProviderInfo* mdp_info = &dump_providers_.find(mdp)->second;
   if (!dump_successful) {
-    LOG(ERROR) << "The memory dumper failed, possibly due to sandboxing "
-                  "(crbug.com/461788), disabling it for current process. Try "
-                  "restarting chrome with the --no-sandbox switch.";
-    dump_providers_.find(mdp)->second.disabled = true;
+    // Disable the MDP if it fails kMaxConsecutiveFailuresCount times
+    // consecutively.
+    mdp_info->consecutive_failures++;
+    if (mdp_info->consecutive_failures >= kMaxConsecutiveFailuresCount) {
+      mdp_info->disabled = true;
+      LOG(ERROR) << "The memory dumper failed, possibly due to sandboxing "
+                    "(crbug.com/461788), disabling it for current process. Try "
+                    "restarting chrome with the --no-sandbox switch.";
+    }
+  } else {
+    mdp_info->consecutive_failures = 0;
   }

[picksi, 2015/06/16 12:56:44]

nit: Removing the '!' and swapping the if/else bodies around would make this
(slightly) more readable.

[ssid, 2015/06/16 14:52:35]

Done.

   return dump_successful;
 }
 
 // This is posted to arbitrary threads as a continuation of CreateProcessDump(),
 // when one or more MemoryDumpProvider(s) require the OnMemoryDump() call to
 // happen on a different thread.
 void MemoryDumpManager::ContinueAsyncProcessDump(
     MemoryDumpProvider* mdp,
     scoped_refptr<ProcessMemoryDumpHolder> pmd_holder) {
   bool should_finalize_dump = false;
@@ skipping 31 matching lines @@
 
   // There is no point starting the tracing without a delegate.
   if (!enabled || !delegate_) {
     // Disable all the providers.
     for (auto it = dump_providers_.begin(); it != dump_providers_.end(); ++it)
       it->second.disabled = true;
     return;
   }
 
   session_state_ = new MemoryDumpSessionState();
-  for (auto it = dump_providers_.begin(); it != dump_providers_.end(); ++it)
+  for (auto it = dump_providers_.begin(); it != dump_providers_.end(); ++it) {
     it->second.disabled = false;
+    it->second.consecutive_failures = 0;
+  }
 
   subtle::NoBarrier_Store(&memory_tracing_enabled_, 1);
 
   if (delegate_->IsCoordinatorProcess()) {
     periodic_dump_timer_.Start(FROM_HERE,
                                TimeDelta::FromSeconds(kDumpIntervalSeconds),
                                base::Bind(&RequestPeriodicGlobalDump));
   }
 }
 
 void MemoryDumpManager::OnTraceLogDisabled() {
   AutoLock lock(lock_);
   periodic_dump_timer_.Stop();
   subtle::NoBarrier_Store(&memory_tracing_enabled_, 0);
   session_state_ = nullptr;
 }
 
 MemoryDumpManager::MemoryDumpProviderInfo::MemoryDumpProviderInfo(
     const scoped_refptr<SingleThreadTaskRunner>& task_runner)
-    : task_runner(task_runner), disabled(false) {
+    : task_runner(task_runner), consecutive_failures(0), disabled(false) {
 }
 MemoryDumpManager::MemoryDumpProviderInfo::~MemoryDumpProviderInfo() {
 }
 
 }  // namespace trace_event
 }  // namespace base