Index: base/trace_event/memory_dump_manager.cc |
diff --git a/base/trace_event/memory_dump_manager.cc b/base/trace_event/memory_dump_manager.cc |
index 10e5d1b13f6c6e22756b73e5e4da0c73dd1533a8..d449d78cd5c8c132666c82b4f02eb5a7b12a6228 100644 |
--- a/base/trace_event/memory_dump_manager.cc |
+++ b/base/trace_event/memory_dump_manager.cc |
@@ -489,6 +489,24 @@ bool MemoryDumpManager::IsDumpProviderRegisteredForTesting( |
return false; |
} |
+scoped_refptr<base::SingleThreadTaskRunner> MemoryDumpManager::GetTaskRunner() { |
+ lock_.AssertAcquired(); |
+ |
+ if (dump_thread_) |
+ return dump_thread_->task_runner(); |
+ |
+ // Spin-up the thread used to invoke unbound dump providers. |
+ std::unique_ptr<Thread> dump_thread(new Thread("MemoryInfra")); |
+ if (!dump_thread->Start()) { |
Primiano Tucci (use gerrit)
2017/04/24 15:30:58
I'd just violently CHECK(started) here.
Trying to
hjd
2017/04/24 16:27:59
Done.
|
+ LOG(ERROR) << "Failed to start the memory-infra thread for tracing"; |
+ NOTREACHED(); |
+ } |
+ |
+ dump_thread_ = std::move(dump_thread); |
+ |
+ return dump_thread_->task_runner(); |
+} |
+ |
void MemoryDumpManager::CreateProcessDump( |
const MemoryDumpRequestArgs& args, |
const ProcessMemoryDumpCallback& callback) { |
@@ -512,12 +530,8 @@ void MemoryDumpManager::CreateProcessDump( |
{ |
AutoLock lock(lock_); |
- // |dump_thread_| can be nullptr is tracing was disabled before reaching |
- // here. SetupNextMemoryDump() is robust enough to tolerate it and will |
- // NACK the dump. |
pmd_async_state.reset(new ProcessMemoryDumpAsyncState( |
- args, dump_providers_, session_state_, callback, |
- dump_thread_ ? dump_thread_->task_runner() : nullptr)); |
+ args, dump_providers_, session_state_, callback, GetTaskRunner())); |
// Safety check to prevent reaching here without calling RequestGlobalDump, |
// with disallowed modes. If |session_state_| is null then tracing is |
@@ -551,22 +565,22 @@ void MemoryDumpManager::SetupNextMemoryDump( |
// (for discounting trace memory overhead) while holding the |lock_|. |
TraceLog::GetInstance()->InitializeThreadLocalEventBufferIfSupported(); |
- // |dump_thread_| might be destroyed before getting this point. |
- // It means that tracing was disabled right before starting this dump. |
- // Anyway either tracing is stopped or this was the last hop, create a trace |
+ // MDM might have been disabled before getting to this point. |
+ // Anyway either MDM is disabled or this was the last hop, create a trace |
// event, add it to the trace and finalize process dump invoking the callback. |
- if (!pmd_async_state->dump_thread_task_runner.get()) { |
+ if (!subtle::NoBarrier_Load(&memory_tracing_enabled_)) { |
Primiano Tucci (use gerrit)
2017/04/24 15:30:58
I think this extra check is superfluous. The callb
hjd
2017/04/24 16:27:59
Removing this causes a test (MemoryDumpManagerTest
ssid
2017/04/25 03:21:47
I agree with primiano. We could get to a state whe
|
if (pmd_async_state->pending_dump_providers.empty()) { |
- VLOG(1) << kLogPrefix << " failed because dump thread was destroyed" |
+ VLOG(1) << kLogPrefix << " failed because MemoryDumpManager was disabled" |
<< " before finalizing the dump"; |
} else { |
- VLOG(1) << kLogPrefix << " failed because dump thread was destroyed" |
+ VLOG(1) << kLogPrefix << " failed because MemoryDumpManager was disabled" |
<< " before dumping " |
<< pmd_async_state->pending_dump_providers.back().get()->name; |
} |
pmd_async_state->dump_successful = false; |
pmd_async_state->pending_dump_providers.clear(); |
} |
+ |
if (pmd_async_state->pending_dump_providers.empty()) |
return FinalizeDumpAndAddToTrace(std::move(pmd_async_state)); |
@@ -585,7 +599,7 @@ void MemoryDumpManager::SetupNextMemoryDump( |
} |
// If the dump provider did not specify a task runner affinity, dump on |
- // |dump_thread_| which is already checked above for presence. |
+ // |dump_thread_|. |
SequencedTaskRunner* task_runner = mdpinfo->task_runner.get(); |
if (!task_runner) { |
DCHECK(mdpinfo->options.dumps_on_single_thread_task_runner); |
@@ -786,12 +800,6 @@ void MemoryDumpManager::FinalizeDumpAndAddToTrace( |
void MemoryDumpManager::Enable( |
const TraceConfig::MemoryDumpConfig& memory_dump_config) { |
- // Spin-up the thread used to invoke unbound dump providers. |
- std::unique_ptr<Thread> dump_thread(new Thread("MemoryInfra")); |
- if (!dump_thread->Start()) { |
- LOG(ERROR) << "Failed to start the memory-infra thread for tracing"; |
- return; |
- } |
scoped_refptr<MemoryDumpSessionState> session_state = |
new MemoryDumpSessionState; |
@@ -826,9 +834,6 @@ void MemoryDumpManager::Enable( |
DCHECK(delegate_); // At this point we must have a delegate. |
session_state_ = session_state; |
- DCHECK(!dump_thread_); |
- dump_thread_ = std::move(dump_thread); |
- |
subtle::NoBarrier_Store(&memory_tracing_enabled_, 1); |
MemoryDumpScheduler::Config periodic_config; |
@@ -851,7 +856,7 @@ void MemoryDumpManager::Enable( |
MemoryPeakDetector::GetInstance()->Setup( |
BindRepeating(&MemoryDumpManager::GetDumpProvidersForPolling, |
Unretained(this)), |
- dump_thread_->task_runner(), |
+ GetTaskRunner(), |
BindRepeating(&OnPeakDetected, trigger.level_of_detail)); |
MemoryPeakDetector::Config peak_config; |
@@ -864,7 +869,7 @@ void MemoryDumpManager::Enable( |
// When peak detection is enabled, trigger a dump straight away as it |
// gives a good reference point for analyzing the trace. |
if (delegate_->IsCoordinator()) { |
- dump_thread_->task_runner()->PostTask( |
+ GetTaskRunner()->PostTask( |
FROM_HERE, BindRepeating(&OnPeakDetected, trigger.level_of_detail)); |
} |
} |
@@ -872,8 +877,7 @@ void MemoryDumpManager::Enable( |
// Only coordinator process triggers periodic global memory dumps. |
if (delegate_->IsCoordinator() && !periodic_config.triggers.empty()) { |
- MemoryDumpScheduler::GetInstance()->Start(periodic_config, |
- dump_thread_->task_runner()); |
+ MemoryDumpScheduler::GetInstance()->Start(periodic_config, GetTaskRunner()); |
} |
} |
@@ -884,19 +888,12 @@ void MemoryDumpManager::Disable() { |
if (!subtle::NoBarrier_Load(&memory_tracing_enabled_)) |
return; |
subtle::NoBarrier_Store(&memory_tracing_enabled_, 0); |
- std::unique_ptr<Thread> dump_thread; |
{ |
AutoLock lock(lock_); |
MemoryDumpScheduler::GetInstance()->Stop(); |
MemoryPeakDetector::GetInstance()->TearDown(); |
- dump_thread = std::move(dump_thread_); |
session_state_ = nullptr; |
} |
- |
- // Thread stops are blocking and must be performed outside of the |lock_| |
- // or will deadlock (e.g., if SetupNextMemoryDump() tries to acquire it). |
- if (dump_thread) |
- dump_thread->Stop(); |
Primiano Tucci (use gerrit)
2017/04/24 15:30:58
\o/ This, as a side effect, should fix crbug.com/7
hjd
2017/04/24 16:27:59
Done.
ssid
2017/04/25 03:21:47
Yay
|
} |
bool MemoryDumpManager::IsDumpModeAllowed(MemoryDumpLevelOfDetail dump_mode) { |