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

Side by Side Diff: base/trace_event/memory_dump_manager.cc

Issue 2592543002: Make memory-infra tests register / unregister with thread safety (Closed)
Patch Set: fix tests. Created 4 years 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 unified diff | Download patch
« no previous file with comments | « no previous file | base/trace_event/memory_dump_manager_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2015 The Chromium Authors. All rights reserved. 1 // Copyright 2015 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "base/trace_event/memory_dump_manager.h" 5 #include "base/trace_event/memory_dump_manager.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <utility> 8 #include <utility>
9 9
10 #include "base/allocator/features.h" 10 #include "base/allocator/features.h"
(...skipping 259 matching lines...) Expand 10 before | Expand all | Expand 10 after
270 scoped_refptr<SequencedTaskRunner> task_runner, 270 scoped_refptr<SequencedTaskRunner> task_runner,
271 const MemoryDumpProvider::Options& options) { 271 const MemoryDumpProvider::Options& options) {
272 if (dumper_registrations_ignored_for_testing_) 272 if (dumper_registrations_ignored_for_testing_)
273 return; 273 return;
274 274
275 bool whitelisted_for_background_mode = IsMemoryDumpProviderWhitelisted(name); 275 bool whitelisted_for_background_mode = IsMemoryDumpProviderWhitelisted(name);
276 scoped_refptr<MemoryDumpProviderInfo> mdpinfo = 276 scoped_refptr<MemoryDumpProviderInfo> mdpinfo =
277 new MemoryDumpProviderInfo(mdp, name, std::move(task_runner), options, 277 new MemoryDumpProviderInfo(mdp, name, std::move(task_runner), options,
278 whitelisted_for_background_mode); 278 whitelisted_for_background_mode);
279 279
280 if (options.is_fast_polling_supported) {
281 DCHECK(!mdpinfo->task_runner) << "MemoryDumpProviders capable of fast "
282 "polling must NOT be thread bound.";
283 }
284
280 { 285 {
281 AutoLock lock(lock_); 286 AutoLock lock(lock_);
282 bool already_registered = !dump_providers_.insert(mdpinfo).second; 287 bool already_registered = !dump_providers_.insert(mdpinfo).second;
283 // This actually happens in some tests which don't have a clean tear-down 288 // This actually happens in some tests which don't have a clean tear-down
284 // path for RenderThreadImpl::Init(). 289 // path for RenderThreadImpl::Init().
285 if (already_registered) 290 if (already_registered)
286 return; 291 return;
287 292
288 // The list of polling MDPs is populated OnTraceLogEnabled(). This code 293 // The list of polling MDPs is populated OnTraceLogEnabled(). This code
289 // deals with the case of a MDP capable of fast polling that is registered 294 // deals with the case of a MDP capable of fast polling that is registered
(...skipping 37 matching lines...) Expand 10 before | Expand all | Expand 10 after
327 return; // Not registered / already unregistered. 332 return; // Not registered / already unregistered.
328 333
329 if (take_mdp_ownership_and_delete_async) { 334 if (take_mdp_ownership_and_delete_async) {
330 // The MDP will be deleted whenever the MDPInfo struct will, that is either: 335 // The MDP will be deleted whenever the MDPInfo struct will, that is either:
331 // - At the end of this function, if no dump is in progress. 336 // - At the end of this function, if no dump is in progress.
332 // - Either in SetupNextMemoryDump() or InvokeOnMemoryDump() when MDPInfo is 337 // - Either in SetupNextMemoryDump() or InvokeOnMemoryDump() when MDPInfo is
333 // removed from |pending_dump_providers|. 338 // removed from |pending_dump_providers|.
334 // - When the provider is removed from |dump_providers_for_polling_|. 339 // - When the provider is removed from |dump_providers_for_polling_|.
335 DCHECK(!(*mdp_iter)->owned_dump_provider); 340 DCHECK(!(*mdp_iter)->owned_dump_provider);
336 (*mdp_iter)->owned_dump_provider = std::move(owned_mdp); 341 (*mdp_iter)->owned_dump_provider = std::move(owned_mdp);
337 } else if (subtle::NoBarrier_Load(&memory_tracing_enabled_)) { 342 } else {
338 // If you hit this DCHECK, your dump provider has a bug. 343 // If you hit this DCHECK, your dump provider has a bug.
339 // Unregistration of a MemoryDumpProvider is safe only if: 344 // Unregistration of a MemoryDumpProvider is safe only if:
340 // - The MDP has specified a sequenced task runner affinity AND the 345 // - The MDP has specified a sequenced task runner affinity AND the
341 // unregistration happens on the same task runner. So that the MDP cannot 346 // unregistration happens on the same task runner. So that the MDP cannot
342 // unregister and be in the middle of a OnMemoryDump() at the same time. 347 // unregister and be in the middle of a OnMemoryDump() at the same time.
343 // - The MDP has NOT specified a task runner affinity and its ownership is 348 // - The MDP has NOT specified a task runner affinity and its ownership is
344 // transferred via UnregisterAndDeleteDumpProviderSoon(). 349 // transferred via UnregisterAndDeleteDumpProviderSoon().
345 // In all the other cases, it is not possible to guarantee that the 350 // In all the other cases, it is not possible to guarantee that the
346 // unregistration will not race with OnMemoryDump() calls. 351 // unregistration will not race with OnMemoryDump() calls.
347 DCHECK((*mdp_iter)->task_runner && 352 DCHECK((*mdp_iter)->task_runner &&
348 (*mdp_iter)->task_runner->RunsTasksOnCurrentThread()) 353 (*mdp_iter)->task_runner->RunsTasksOnCurrentThread())
349 << "MemoryDumpProvider \"" << (*mdp_iter)->name << "\" attempted to " 354 << "MemoryDumpProvider \"" << (*mdp_iter)->name << "\" attempted to "
350 << "unregister itself in a racy way. Please file a crbug."; 355 << "unregister itself in a racy way. Please file a crbug.";
351 } 356 }
352 357
353 if ((*mdp_iter)->options.is_fast_polling_supported && dump_thread_) { 358 if ((*mdp_iter)->options.is_fast_polling_supported && dump_thread_) {
354 DCHECK(take_mdp_ownership_and_delete_async)
Primiano Tucci (use gerrit) 2016/12/21 11:44:25 I'd still kjeep the DCHECK here, without the strin
ssid 2016/12/21 19:55:03 Done.
355 << "MemoryDumpProviders capable of fast polling must NOT be thread "
356 "bound and hence must be destroyed using "
357 "UnregisterAndDeleteDumpProviderSoon()";
358 dump_thread_->task_runner()->PostTask( 359 dump_thread_->task_runner()->PostTask(
359 FROM_HERE, Bind(&MemoryDumpManager::UnregisterPollingMDPOnDumpThread, 360 FROM_HERE, Bind(&MemoryDumpManager::UnregisterPollingMDPOnDumpThread,
360 Unretained(this), *mdp_iter)); 361 Unretained(this), *mdp_iter));
361 } 362 }
362 363
363 // The MDPInfo instance can still be referenced by the 364 // The MDPInfo instance can still be referenced by the
364 // |ProcessMemoryDumpAsyncState.pending_dump_providers|. For this reason 365 // |ProcessMemoryDumpAsyncState.pending_dump_providers|. For this reason
365 // the MDPInfo is flagged as disabled. It will cause InvokeOnMemoryDump() 366 // the MDPInfo is flagged as disabled. It will cause InvokeOnMemoryDump()
366 // to just skip it, without actually invoking the |mdp|, which might be 367 // to just skip it, without actually invoking the |mdp|, which might be
367 // destroyed by the caller soon after this method returns. 368 // destroyed by the caller soon after this method returns.
368 (*mdp_iter)->disabled = true; 369 (*mdp_iter)->disabled = true;
369 dump_providers_.erase(mdp_iter); 370 dump_providers_.erase(mdp_iter);
370 } 371 }
371 372
372 void MemoryDumpManager::RegisterPollingMDPOnDumpThread( 373 void MemoryDumpManager::RegisterPollingMDPOnDumpThread(
373 scoped_refptr<MemoryDumpManager::MemoryDumpProviderInfo> mdpinfo) { 374 scoped_refptr<MemoryDumpManager::MemoryDumpProviderInfo> mdpinfo) {
374 DCHECK(!mdpinfo->task_runner);
375 AutoLock lock(lock_); 375 AutoLock lock(lock_);
376 dump_providers_for_polling_.insert(mdpinfo); 376 dump_providers_for_polling_.insert(mdpinfo);
377 } 377 }
378 378
379 void MemoryDumpManager::UnregisterPollingMDPOnDumpThread( 379 void MemoryDumpManager::UnregisterPollingMDPOnDumpThread(
380 scoped_refptr<MemoryDumpManager::MemoryDumpProviderInfo> mdpinfo) { 380 scoped_refptr<MemoryDumpManager::MemoryDumpProviderInfo> mdpinfo) {
381 mdpinfo->dump_provider->SuspendFastMemoryPolling(); 381 mdpinfo->dump_provider->SuspendFastMemoryPolling();
382 382
383 AutoLock lock(lock_); 383 AutoLock lock(lock_);
384 dump_providers_for_polling_.erase(mdpinfo); 384 dump_providers_for_polling_.erase(mdpinfo);
(...skipping 570 matching lines...) Expand 10 before | Expand all | Expand 10 after
955 if (heavy_dump_rate_ > 0 && periodic_dumps_count_ % heavy_dump_rate_ == 0) 955 if (heavy_dump_rate_ > 0 && periodic_dumps_count_ % heavy_dump_rate_ == 0)
956 level_of_detail = MemoryDumpLevelOfDetail::DETAILED; 956 level_of_detail = MemoryDumpLevelOfDetail::DETAILED;
957 ++periodic_dumps_count_; 957 ++periodic_dumps_count_;
958 958
959 MemoryDumpManager::GetInstance()->RequestGlobalDump( 959 MemoryDumpManager::GetInstance()->RequestGlobalDump(
960 MemoryDumpType::PERIODIC_INTERVAL, level_of_detail); 960 MemoryDumpType::PERIODIC_INTERVAL, level_of_detail);
961 } 961 }
962 962
963 } // namespace trace_event 963 } // namespace trace_event
964 } // namespace base 964 } // namespace base
OLDNEW
« no previous file with comments | « no previous file | base/trace_event/memory_dump_manager_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698