Support parallel captures from the StackSamplingProfiler.

base/profiler/stack_sampling_profiler_unittest.cc

Index: base/profiler/stack_sampling_profiler_unittest.cc
diff --git a/base/profiler/stack_sampling_profiler_unittest.cc b/base/profiler/stack_sampling_profiler_unittest.cc
index 16205ac2b09f22e25975cda252c8306ba6eb918a..96ff7deb461686f1cda790d99d30edd63a5e58aa 100644
--- a/base/profiler/stack_sampling_profiler_unittest.cc
+++ b/base/profiler/stack_sampling_profiler_unittest.cc
@@ -59,6 +59,39 @@ using CallStackProfiles = StackSamplingProfiler::CallStackProfiles;
 
 namespace {
 
+// Test delegate that signals when the thread-under-test has exited.
+class ThreadLifetimeSignaler : public NativeStackSamplerTestDelegate {
+ public:
+  ThreadLifetimeSignaler(WaitableEvent* thread_started,
+                         WaitableEvent* thread_sampled,
+                         WaitableEvent* thread_exited)
+      : thread_started_(thread_started),
+        thread_sampled_(thread_sampled),
+        thread_exited_(thread_exited) {}
+
+  void OnPostRecordSample(NativeStackSampler::ThreadState state) override {
+    if (state == NativeStackSampler::THREAD_RUNNING) {
+      if (thread_started_) {
+        thread_started_->Signal();
+        thread_started_ = nullptr;
+      }
+    }
+    if (thread_sampled_)
+      thread_sampled_->Signal();
+    if (state == NativeStackSampler::THREAD_EXITED) {
+      if (thread_exited_) {
+        thread_exited_->Signal();
+        thread_exited_ = nullptr;
+      }
+    }
+  }
+
+ private:
+  WaitableEvent* thread_started_;
+  WaitableEvent* thread_sampled_;
+  WaitableEvent* thread_exited_;
+};
+
 // Configuration for the frames that appear on the stack.
 struct StackConfiguration {
   enum Config { NORMAL, WITH_ALLOCA, WITH_OTHER_LIBRARY };
@@ -444,9 +477,9 @@ void TestLibraryUnload(bool wait_until_unloaded) {
     StackCopiedSignaler(WaitableEvent* stack_copied,
                         WaitableEvent* start_stack_walk,
                         bool wait_to_walk_stack)
-        : stack_copied_(stack_copied), start_stack_walk_(start_stack_walk),
-          wait_to_walk_stack_(wait_to_walk_stack) {
-    }
+        : stack_copied_(stack_copied),
+          start_stack_walk_(start_stack_walk),
+          wait_to_walk_stack_(wait_to_walk_stack) {}
 
     void OnPreStackWalk() override {
       stack_copied_->Signal();
@@ -623,6 +656,18 @@ TEST(StackSamplingProfilerTest, MAYBE_Basic) {
   FilePath executable_path;
   EXPECT_TRUE(PathService::Get(FILE_EXE, &executable_path));
   EXPECT_EQ(executable_path, profile.modules[loc->module_index].filename);
+
+  // Ensure the sampling thread is still running at this point.
+  ASSERT_TRUE(StackSamplingProfiler::IsSamplingThreadRunningForTesting());
+
+  // Shutdown the sampler as though the process were about to exit.
+  StackSamplingProfiler::Shutdown();
+  EXPECT_FALSE(StackSamplingProfiler::IsSamplingThreadRunningForTesting());
+
+  // Shutdown is a permanent thing in general but this needs to be undone so
+  // other tests can run.
+  StackSamplingProfiler::UndoShutdownForTesting();
+  EXPECT_FALSE(StackSamplingProfiler::IsSamplingThreadRunningForTesting());
 }
 
 // Checks that annotations are recorded in samples.
@@ -720,35 +765,6 @@ TEST(StackSamplingProfilerTest, MAYBE_Alloca) {
       << FormatSampleForDiagnosticOutput(sample, profile.modules);
 }
 
-// Checks that the fire-and-forget interface works.
-#if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
-#define MAYBE_StartAndRunAsync StartAndRunAsync
-#else
-#define MAYBE_StartAndRunAsync DISABLED_StartAndRunAsync
-#endif
-TEST(StackSamplingProfilerTest, MAYBE_StartAndRunAsync) {
-  // StartAndRunAsync requires the caller to have a message loop.
-  MessageLoop message_loop;
-
-  SamplingParams params;
-  params.samples_per_burst = 1;
-
-  CallStackProfiles profiles;
-  WithTargetThread([&params, &profiles](PlatformThreadId target_thread_id) {
-    WaitableEvent sampling_thread_completed(
-        WaitableEvent::ResetPolicy::AUTOMATIC,
-        WaitableEvent::InitialState::NOT_SIGNALED);
-    const StackSamplingProfiler::CompletedCallback callback =
-        Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles),
-             Unretained(&sampling_thread_completed));
-    StackSamplingProfiler::StartAndRunAsync(target_thread_id, params, callback);
-    RunLoop().RunUntilIdle();
-    sampling_thread_completed.Wait();
-  });
-
-  ASSERT_EQ(1u, profiles.size());
-}
-
 // Checks that the expected number of profiles and samples are present in the
 // call stack profiles produced.
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
@@ -853,6 +869,70 @@ TEST(StackSamplingProfilerTest, MAYBE_DestroyProfilerWhileProfiling) {
   });
 }
 
+// Checks that we can destroy the thread under test while profiling.
+// This isn't officially supported but is expected to work under Windows.
+#if defined(STACK_SAMPLING_PROFILER_SUPPORTED) && defined(OS_WIN)
+#define MAYBE_DestroyThreadWhileProfiling DestroyThreadWhileProfiling
+#else
+#define MAYBE_DestroyThreadWhileProfiling DISABLED_DestroyThreadWhileProfiling
+#endif
+TEST(StackSamplingProfilerTest, MAYBE_DestroyThreadWhileProfiling) {

[Mike Wittman, 2017/02/13 22:35:57]

This test can be removed since it's no longer the responsibility of the
StackSamplingProfiler to manage thread exit.

[bcwhite, 2017/02/14 14:33:02]

Done.

+  // Set up a thread for testing.
+  StackConfiguration stack_config(StackConfiguration::NORMAL);
+  TargetThread target_thread(stack_config);
+  PlatformThreadHandle target_thread_handle;
+  ASSERT_TRUE(PlatformThread::Create(0, &target_thread, &target_thread_handle));
+
+  // Set up the sampler: 5 samples, 1 second apart
+  SamplingParams params;
+  params.sampling_interval = TimeDelta::FromSeconds(1);
+  params.samples_per_burst = 5;
+  std::vector<CallStackProfile> profiles;
+  WaitableEvent sampling_thread_completed(
+      WaitableEvent::ResetPolicy::MANUAL,
+      WaitableEvent::InitialState::NOT_SIGNALED);
+  const StackSamplingProfiler::CompletedCallback callback =
+      Bind(&SaveProfilesAndSignalEvent, Unretained(&profiles),
+           Unretained(&sampling_thread_completed));
+
+  // Set up events to detect thread lifetime.
+  WaitableEvent thread_started(WaitableEvent::ResetPolicy::MANUAL,
+                               WaitableEvent::InitialState::NOT_SIGNALED);
+  WaitableEvent thread_sampled(WaitableEvent::ResetPolicy::MANUAL,
+                               WaitableEvent::InitialState::NOT_SIGNALED);
+  WaitableEvent thread_exited(WaitableEvent::ResetPolicy::MANUAL,
+                              WaitableEvent::InitialState::NOT_SIGNALED);
+  ThreadLifetimeSignaler test_delegate(&thread_started, &thread_sampled,
+                                       &thread_exited);
+
+  // Start the thread and the sampler and wait a short while.
+  target_thread.WaitForThreadStart();
+  StackSamplingProfiler profiler(target_thread.id(), params, callback,
+                                 &test_delegate);
+  profiler.Start();
+
+  // Delay until the thread gets started and a sample is collected.
+  thread_started.Wait();
+
+  // Stop the target thread.
+  target_thread.SignalThreadToFinish();
+  PlatformThread::Join(target_thread_handle);
+
+  // Wait for one more sampling attempt.
+  thread_sampled.Reset();
+  thread_sampled.Wait();
+
+  // Stop sampling and wait for it to finish.
+  profiler.Stop();
+  sampling_thread_completed.Wait();
+
+  // There should be one profile in which the first sample has frames and
+  // the last does not.
+  ASSERT_EQ(1U, profiles.size());
+  EXPECT_NE(0U, profiles[0].samples.front().frames.size());
+  EXPECT_EQ(0U, profiles[0].samples.back().frames.size());
+}
+
 // Checks that the same profiler may be run multiple times.
 #if defined(STACK_SAMPLING_PROFILER_SUPPORTED)
 #define MAYBE_CanRunMultipleTimes CanRunMultipleTimes
@@ -860,6 +940,8 @@ TEST(StackSamplingProfilerTest, MAYBE_DestroyProfilerWhileProfiling) {
 #define MAYBE_CanRunMultipleTimes DISABLED_CanRunMultipleTimes
 #endif
 TEST(StackSamplingProfilerTest, MAYBE_CanRunMultipleTimes) {
+  StackSamplingProfiler::SetSamplingThreadIdleShutdownTimeForTesting(0);
+
   SamplingParams params;
   params.sampling_interval = TimeDelta::FromMilliseconds(0);
   params.samples_per_burst = 1;
@@ -871,6 +953,21 @@ TEST(StackSamplingProfilerTest, MAYBE_CanRunMultipleTimes) {
   profiles.clear();
   CaptureProfiles(params, AVeryLongTimeDelta(), &profiles);
   ASSERT_EQ(1u, profiles.size());
+
+  // Capture thread should still be running at this point.
+  ASSERT_TRUE(StackSamplingProfiler::IsSamplingThreadRunningForTesting());
+
+  // Initiate an "idle" shutdown.  The task will be run immediately but on
+  // another thread so wait for it to complete.
+  StackSamplingProfiler::InitiateSamplingThreadIdleShutdownForTesting();
+  while (StackSamplingProfiler::IsSamplingThreadRunningForTesting())
+    PlatformThread::YieldCurrentThread();
+
+  // Ensure another capture will start the sampling thread and run.
+  profiles.clear();
+  CaptureProfiles(params, AVeryLongTimeDelta(), &profiles);
+  ASSERT_EQ(1u, profiles.size());
+  EXPECT_TRUE(StackSamplingProfiler::IsSamplingThreadRunningForTesting());
 }
 
 // Checks that requests to start profiling while another profile is taking place

[Mike Wittman, 2017/02/13 22:35:57]

This comment needs updating.

[bcwhite, 2017/02/14 14:33:02]

Done.

@@ -884,11 +981,12 @@ TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling) {
   WithTargetThread([](PlatformThreadId target_thread_id) {
     SamplingParams params[2];
     params[0].initial_delay = TimeDelta::FromMilliseconds(10);
-    params[0].sampling_interval = TimeDelta::FromMilliseconds(0);
-    params[0].samples_per_burst = 1;
+    params[0].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[0].samples_per_burst = 10;
 
-    params[1].sampling_interval = TimeDelta::FromMilliseconds(0);
-    params[1].samples_per_burst = 1;
+    params[0].initial_delay = TimeDelta::FromMilliseconds(10);
+    params[1].sampling_interval = TimeDelta::FromMilliseconds(1);
+    params[1].samples_per_burst = 10;
 
     CallStackProfiles profiles[2];
     std::vector<std::unique_ptr<WaitableEvent>> sampling_completed(2);
@@ -919,14 +1017,9 @@ TEST(StackSamplingProfilerTest, MAYBE_ConcurrentProfiling) {
     EXPECT_EQ(1u, profiles[completed_profiler].size());
 
     size_t other_profiler = 1 - completed_profiler;
-    // Give the other profiler a chance to run and observe that it hasn't.
-    EXPECT_FALSE(sampling_completed[other_profiler]->TimedWait(
-        TimeDelta::FromMilliseconds(25)));
-
-    // Start the other profiler again and it should run.
-    profiler[other_profiler]->Start();
-    sampling_completed[other_profiler]->Wait();
-    EXPECT_EQ(1u, profiles[other_profiler].size());
+    // Give the other profiler a chance to finish and verify it does no.

[Mike Wittman, 2017/02/13 22:35:57]

nit: does so

[bcwhite, 2017/02/14 14:33:02]

Done.

+    EXPECT_TRUE(sampling_completed[other_profiler]->TimedWait(
+        TimeDelta::FromMilliseconds(250)));
   });
 }