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

Unified Diff: base/trace_event/memory_dump_manager.cc

Issue 2592233002: [memory-infra] Make thread check at unregistration stricter for new dump providers (Closed)
Patch Set: Use set instead of searching string list Created 3 years, 11 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/trace_event/memory_dump_manager.h ('k') | base/trace_event/memory_dump_manager_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 dd67c3155d99c73b07a26bc348eaabdbd7d3afd4..ae08d44bdd223fb55daf373013489f6f2604ed7d 100644
--- a/base/trace_event/memory_dump_manager.cc
+++ b/base/trace_event/memory_dump_manager.cc
@@ -47,6 +47,45 @@ const unsigned char kTraceEventArgTypes[] = {TRACE_VALUE_TYPE_CONVERTABLE};
StaticAtomicSequenceNumber g_next_guid;
MemoryDumpManager* g_instance_for_testing = nullptr;
+// The list of names of dump providers that are blacklisted from strict thread
+// affinity check on unregistration. These providers could potentially cause
+// crashes on build bots if they do not unregister on right thread.
+// TODO(ssid): Fix all the dump providers to unregister if needed and clear the
+// blacklist, crbug.com/643438.
+const char* const kStrictThreadCheckBlacklist[] = {
+ "android::ResourceManagerImpl",
+ "AndroidGraphics",
+ "BrowserGpuMemoryBufferManager",
+ "ClientDiscardableSharedMemoryManager",
+ "ContextProviderCommandBuffer",
+ "DOMStorage",
+ "DiscardableSharedMemoryManager",
+ "FontCaches",
+ "GpuMemoryBufferVideoFramePool",
+ "IndexedDBBackingStore",
+ "LeveldbValueStore",
+ "MemoryCache",
+ "Skia",
+ "Sql",
+ "ThreadLocalEventBuffer",
+ "TraceLog",
+ "URLRequestContext",
+ "V8Isolate",
+ "VpxVideoDecoder",
+ "cc::ResourcePool",
+ "cc::ResourceProvider",
+ "cc::SoftwareImageDecodeCache",
+ "cc::StagingBufferPool",
+ "gpu::BufferManager",
+ "gpu::MappedMemoryManager",
+ "gpu::RenderbufferManager",
+ "gpu::TextureManager",
+ "gpu::TransferBufferManager",
+ "sql::Connection",
+ "SyncDirectory",
+ "BlacklistTestDumpProvider" // for testing
+};
+
// Callback wrapper to hook upon the completion of RequestGlobalDump() and
// inject trace markers.
void OnGlobalDumpDone(MemoryDumpCallback wrapped_callback,
@@ -138,6 +177,9 @@ MemoryDumpManager::MemoryDumpManager()
// At this point the command line may not be initialized but we try to
// enable the heap profiler to capture allocations as soon as possible.
EnableHeapProfilingIfNeeded();
+
+ strict_thread_check_blacklist_.insert(std::begin(kStrictThreadCheckBlacklist),
+ std::end(kStrictThreadCheckBlacklist));
}
MemoryDumpManager::~MemoryDumpManager() {
@@ -339,7 +381,15 @@ void MemoryDumpManager::UnregisterDumpProviderInternal(
// - When the provider is removed from |dump_providers_for_polling_|.
DCHECK(!(*mdp_iter)->owned_dump_provider);
(*mdp_iter)->owned_dump_provider = std::move(owned_mdp);
- } else if (subtle::NoBarrier_Load(&memory_tracing_enabled_)) {
+ } else if (strict_thread_check_blacklist_.count((*mdp_iter)->name) == 0 ||
+ subtle::NoBarrier_Load(&memory_tracing_enabled_)) {
+ // If dump provider's name is on |strict_thread_check_blacklist_|, then the
+ // DCHECK is fired only when tracing is enabled. Otherwise the DCHECK is
+ // fired even when tracing is not enabled (stricter).
+ // TODO(ssid): Remove this condition after removing all the dump providers
+ // in the blacklist and the buildbots are no longer flakily hitting the
+ // DCHECK, crbug.com/643438.
+
// If you hit this DCHECK, your dump provider has a bug.
// Unregistration of a MemoryDumpProvider is safe only if:
// - The MDP has specified a sequenced task runner affinity AND the
« no previous file with comments | « base/trace_event/memory_dump_manager.h ('k') | base/trace_event/memory_dump_manager_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698