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 kMaxFailuresCount = 3;

[Primiano Tucci (use gerrit), 2015/06/10 23:37:07]

+consecutive

[ssid, 2015/06/12 14:37:00]

Done.

   bool dump_successful = mdp->OnMemoryDump(pmd);
   if (!dump_successful) {
     LOG(ERROR) << "The memory dumper failed, possibly due to sandboxing "

[Primiano Tucci (use gerrit), 2015/06/10 23:37:07]

you want to give this message only if it fails N time, not each time

[ssid, 2015/06/12 14:37:00]

Done.

                   "(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 kMaxFailuresCount times consecutively.
+    dump_providers_.find(mdp)->second.failures++;

[Primiano Tucci (use gerrit), 2015/06/10 23:37:07]

Let's just lookup once, and keep the ptr to the mdpinfo

[ssid, 2015/06/12 14:37:00]

Done.

+    if (dump_providers_.find(mdp)->second.failures >= kMaxFailuresCount)
+      dump_providers_.find(mdp)->second.disabled = true;
+  } else {
+    dump_providers_.find(mdp)->second.failures = 0;
   }
   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) {
@@ skipping 33 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)
     it->second.disabled = false;

[Primiano Tucci (use gerrit), 2015/06/10 23:37:07]

and you want to reset the count here, otherwise once they fail will be disabled
forever

[ssid, 2015/06/12 14:37:00]

Done.

 
   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), failures(0), disabled(false) {
 }
 MemoryDumpManager::MemoryDumpProviderInfo::~MemoryDumpProviderInfo() {
 }
 
 }  // namespace trace_event
 }  // namespace base