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

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

Issue 1884573002: Fix race condition when dump providers invoked within destroyed state (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 8 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 unified diff | Download patch
« no previous file with comments | « base/trace_event/memory_dump_manager.cc ('k') | no next file » | 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 <stdint.h> 7 #include <stdint.h>
8 8
9 #include <memory> 9 #include <memory>
10 #include <vector> 10 #include <vector>
(...skipping 99 matching lines...) Expand 10 before | Expand all | Expand 10 after
110 // Promote the CreateProcessDump to public so it can be used by test fixtures. 110 // Promote the CreateProcessDump to public so it can be used by test fixtures.
111 using MemoryDumpManagerDelegate::CreateProcessDump; 111 using MemoryDumpManagerDelegate::CreateProcessDump;
112 }; 112 };
113 113
114 class MockMemoryDumpProvider : public MemoryDumpProvider { 114 class MockMemoryDumpProvider : public MemoryDumpProvider {
115 public: 115 public:
116 MOCK_METHOD0(Destructor, void()); 116 MOCK_METHOD0(Destructor, void());
117 MOCK_METHOD2(OnMemoryDump, 117 MOCK_METHOD2(OnMemoryDump,
118 bool(const MemoryDumpArgs& args, ProcessMemoryDump* pmd)); 118 bool(const MemoryDumpArgs& args, ProcessMemoryDump* pmd));
119 119
120 MockMemoryDumpProvider() : enable_mock_destructor(false) {} 120 MockMemoryDumpProvider() : enable_mock_destructor(false) {
121 ON_CALL(*this, OnMemoryDump(_, _))
122 .WillByDefault(Invoke([](const MemoryDumpArgs&,
123 ProcessMemoryDump* pmd) -> bool {
124 // |session_state| should not be null under any circumstances when
125 // invoking a memory dump. The problem might arise in race conditions
126 // like crbug.com/600570 .
127 EXPECT_TRUE(pmd->session_state().get() != nullptr);
128 return true;
129 }));
130 }
121 ~MockMemoryDumpProvider() override { 131 ~MockMemoryDumpProvider() override {
122 if (enable_mock_destructor) 132 if (enable_mock_destructor)
123 Destructor(); 133 Destructor();
124 } 134 }
125 135
126 bool enable_mock_destructor; 136 bool enable_mock_destructor;
127 }; 137 };
128 138
129 class TestSequencedTaskRunner : public SequencedTaskRunner { 139 class TestSequencedTaskRunner : public SequencedTaskRunner {
130 public: 140 public:
(...skipping 800 matching lines...) Expand 10 before | Expand all | Expand 10 after
931 941
932 EXPECT_FALSE(last_callback_success_); 942 EXPECT_FALSE(last_callback_success_);
933 } 943 }
934 944
935 // Tests against race conditions that can happen if tracing is disabled before 945 // Tests against race conditions that can happen if tracing is disabled before
936 // the CreateProcessDump() call. Real-world regression: crbug.com/580295 . 946 // the CreateProcessDump() call. Real-world regression: crbug.com/580295 .
937 TEST_F(MemoryDumpManagerTest, DisableTracingRightBeforeStartOfDump) { 947 TEST_F(MemoryDumpManagerTest, DisableTracingRightBeforeStartOfDump) {
938 base::WaitableEvent tracing_disabled_event(false, false); 948 base::WaitableEvent tracing_disabled_event(false, false);
939 InitializeMemoryDumpManager(false /* is_coordinator */); 949 InitializeMemoryDumpManager(false /* is_coordinator */);
940 950
941 MockMemoryDumpProvider mdp; 951 std::unique_ptr<Thread> mdp_thread(new Thread("test thread"));
942 RegisterDumpProvider(&mdp); 952 mdp_thread->Start();
953
954 // Create both same-thread MDP and another MDP with dedicated thread
955 MockMemoryDumpProvider mdp1;
956 RegisterDumpProvider(&mdp1);
957 MockMemoryDumpProvider mdp2;
958 RegisterDumpProvider(&mdp2, mdp_thread->task_runner(), kDefaultOptions);
943 EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory); 959 EnableTracingWithLegacyCategories(MemoryDumpManager::kTraceCategory);
944 960
945 EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)) 961 EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _))
946 .WillOnce(Invoke([this](const MemoryDumpRequestArgs& args, 962 .WillOnce(Invoke([this](const MemoryDumpRequestArgs& args,
947 const MemoryDumpCallback& callback) { 963 const MemoryDumpCallback& callback) {
948 DisableTracing(); 964 DisableTracing();
949 delegate_->CreateProcessDump(args, callback); 965 delegate_->CreateProcessDump(args, callback);
950 })); 966 }));
951 967
968 // If tracing is disabled for current session CreateProcessDump() should NOT
969 // request dumps from providers. Real-world regression: crbug.com/600570 .
970 EXPECT_CALL(mdp1, OnMemoryDump(_, _)).Times(0);
971 EXPECT_CALL(mdp2, OnMemoryDump(_, _)).Times(0);
972
952 last_callback_success_ = true; 973 last_callback_success_ = true;
953 RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED, 974 RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
954 MemoryDumpLevelOfDetail::DETAILED); 975 MemoryDumpLevelOfDetail::DETAILED);
955 EXPECT_FALSE(last_callback_success_); 976 EXPECT_FALSE(last_callback_success_);
956 } 977 }
957 978
958 TEST_F(MemoryDumpManagerTest, DumpOnBehalfOfOtherProcess) { 979 TEST_F(MemoryDumpManagerTest, DumpOnBehalfOfOtherProcess) {
959 using trace_analyzer::Query; 980 using trace_analyzer::Query;
960 981
961 InitializeMemoryDumpManager(false /* is_coordinator */); 982 InitializeMemoryDumpManager(false /* is_coordinator */);
(...skipping 111 matching lines...) Expand 10 before | Expand all | Expand 10 after
1073 EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(2); 1094 EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(2);
1074 for (int i = 0; i < 2; ++i) { 1095 for (int i = 0; i < 2; ++i) {
1075 RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED, 1096 RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
1076 MemoryDumpLevelOfDetail::DETAILED); 1097 MemoryDumpLevelOfDetail::DETAILED);
1077 } 1098 }
1078 DisableTracing(); 1099 DisableTracing();
1079 } 1100 }
1080 1101
1081 } // namespace trace_event 1102 } // namespace trace_event
1082 } // namespace base 1103 } // namespace base
OLDNEW
« no previous file with comments | « base/trace_event/memory_dump_manager.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698