Commit fde86224 authored by ssid's avatar ssid Committed by Commit bot

[memory-infra] Remove thread check blacklist

The blacklist was introduced by crrev.com/2592233002 to make
thread checks stricter. The blacklist was to throttle the number
of build bot failures.
There are only few providers left and it causes a need for having
|is_enabled_| in MDM. So, remove this check.

BUG=643438

Review-Url: https://codereview.chromium.org/2844373002
Cr-Commit-Position: refs/heads/master@{#468118}
parent f976fc19
...@@ -54,31 +54,6 @@ namespace { ...@@ -54,31 +54,6 @@ namespace {
StaticAtomicSequenceNumber g_next_guid; StaticAtomicSequenceNumber g_next_guid;
MemoryDumpManager* g_instance_for_testing = nullptr; 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[] = {
"ClientDiscardableSharedMemoryManager",
"ContextProviderCommandBuffer",
"DiscardableSharedMemoryManager",
"FontCaches",
"GpuMemoryBufferVideoFramePool",
"IndexedDBBackingStore",
"Sql",
"ThreadLocalEventBuffer",
"TraceLog",
"URLRequestContext",
"VpxVideoDecoder",
"cc::SoftwareImageDecodeCache",
"cc::StagingBufferPool",
"gpu::BufferManager",
"gpu::MappedMemoryManager",
"gpu::RenderbufferManager",
"BlacklistTestDumpProvider" // for testing
};
// Callback wrapper to hook upon the completion of RequestGlobalDump() and // Callback wrapper to hook upon the completion of RequestGlobalDump() and
// inject trace markers. // inject trace markers.
void OnGlobalDumpDone(GlobalMemoryDumpCallback wrapped_callback, void OnGlobalDumpDone(GlobalMemoryDumpCallback wrapped_callback,
...@@ -191,9 +166,6 @@ MemoryDumpManager::MemoryDumpManager() ...@@ -191,9 +166,6 @@ MemoryDumpManager::MemoryDumpManager()
// At this point the command line may not be initialized but we try to // 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. // enable the heap profiler to capture allocations as soon as possible.
EnableHeapProfilingIfNeeded(); EnableHeapProfilingIfNeeded();
strict_thread_check_blacklist_.insert(std::begin(kStrictThreadCheckBlacklist),
std::end(kStrictThreadCheckBlacklist));
} }
MemoryDumpManager::~MemoryDumpManager() { MemoryDumpManager::~MemoryDumpManager() {
...@@ -398,15 +370,7 @@ void MemoryDumpManager::UnregisterDumpProviderInternal( ...@@ -398,15 +370,7 @@ void MemoryDumpManager::UnregisterDumpProviderInternal(
// - When the provider is removed from other clients (MemoryPeakDetector). // - When the provider is removed from other clients (MemoryPeakDetector).
DCHECK(!(*mdp_iter)->owned_dump_provider); DCHECK(!(*mdp_iter)->owned_dump_provider);
(*mdp_iter)->owned_dump_provider = std::move(owned_mdp); (*mdp_iter)->owned_dump_provider = std::move(owned_mdp);
} else if (strict_thread_check_blacklist_.count((*mdp_iter)->name) == 0 || } else {
subtle::NoBarrier_Load(&is_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. // If you hit this DCHECK, your dump provider has a bug.
// Unregistration of a MemoryDumpProvider is safe only if: // Unregistration of a MemoryDumpProvider is safe only if:
// - The MDP has specified a sequenced task runner affinity AND the // - The MDP has specified a sequenced task runner affinity AND the
......
...@@ -295,11 +295,6 @@ class BASE_EXPORT MemoryDumpManager { ...@@ -295,11 +295,6 @@ class BASE_EXPORT MemoryDumpManager {
// Shared among all the PMDs to keep state scoped to the tracing session. // Shared among all the PMDs to keep state scoped to the tracing session.
scoped_refptr<MemoryDumpSessionState> session_state_; scoped_refptr<MemoryDumpSessionState> session_state_;
// The list of names of dump providers that are blacklisted from strict thread
// affinity check on unregistration.
std::unordered_set<StringPiece, StringPieceHash>
strict_thread_check_blacklist_;
std::unique_ptr<MemoryTracingObserver> tracing_observer_; std::unique_ptr<MemoryTracingObserver> tracing_observer_;
// Function provided by the embedder to handle global dump requests. // Function provided by the embedder to handle global dump requests.
......
...@@ -1270,23 +1270,6 @@ TEST_F(MemoryDumpManagerTest, TestBackgroundTracingSetup) { ...@@ -1270,23 +1270,6 @@ TEST_F(MemoryDumpManagerTest, TestBackgroundTracingSetup) {
DisableTracing(); DisableTracing();
} }
TEST_F(MemoryDumpManagerTest, TestBlacklistedUnsafeUnregistration) {
InitializeMemoryDumpManager(false /* is_coordinator */);
MockMemoryDumpProvider mdp1;
RegisterDumpProvider(&mdp1, nullptr, kDefaultOptions,
"BlacklistTestDumpProvider");
// Not calling UnregisterAndDeleteDumpProviderSoon() should not crash.
mdm_->UnregisterDumpProvider(&mdp1);
Thread thread("test thread");
thread.Start();
RegisterDumpProvider(&mdp1, thread.task_runner(), kDefaultOptions,
"BlacklistTestDumpProvider");
// Unregistering on wrong thread should not crash.
mdm_->UnregisterDumpProvider(&mdp1);
thread.Stop();
}
// Tests that we can manually take a dump without enabling tracing. // Tests that we can manually take a dump without enabling tracing.
TEST_F(MemoryDumpManagerTest, DumpWithTracingDisabled) { TEST_F(MemoryDumpManagerTest, DumpWithTracingDisabled) {
InitializeMemoryDumpManager(false /* is_coordinator */); InitializeMemoryDumpManager(false /* is_coordinator */);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment