Started shutting down the sampler when it gets deleted (flutter#23012)

Author: gaaclarke

ci/licenses_golden/licenses_flutter

@@ -1461,6 +1461,7 @@ FILE: ../../../flutter/shell/platform/windows/window_binding_handler_delegate.h
 FILE: ../../../flutter/shell/platform/windows/window_state.h
 FILE: ../../../flutter/shell/profiling/sampling_profiler.cc
 FILE: ../../../flutter/shell/profiling/sampling_profiler.h
+FILE: ../../../flutter/shell/profiling/sampling_profiler_unittest.cc
 FILE: ../../../flutter/shell/version/version.cc
 FILE: ../../../flutter/shell/version/version.h
 FILE: ../../../flutter/sky/packages/flutter_services/lib/empty.dart

shell/common/BUILD.gn

@@ -254,6 +254,7 @@ if (enable_unittests) {
       ":shell_unittests_fixtures",
       "//flutter/assets",
       "//flutter/common/graphics",
+      "//flutter/shell/profiling:profiling_unittests",
       "//flutter/shell/version",
       "//third_party/googletest:gmock",
     ]

shell/profiling/BUILD.gn

@@ -17,3 +17,12 @@ source_set("profiling") {
 
   deps = _profiler_deps
 }
+
+source_set("profiling_unittests") {
+  testonly = true
+  sources = [ "sampling_profiler_unittest.cc" ]
+  deps = [
+    ":profiling",
+    "//flutter/testing",
+  ]
+}

shell/profiling/sampling_profiler.cc

@@ -16,7 +16,13 @@ SamplingProfiler::SamplingProfiler(
       sampler_(std::move(sampler)),
       num_samples_per_sec_(num_samples_per_sec) {}
 
-void SamplingProfiler::Start() const {
+SamplingProfiler::~SamplingProfiler() {
+  if (is_running_) {
+    Stop();
+  }
+}
+
+void SamplingProfiler::Start() {
   if (!profiler_task_runner_) {
     return;
   }
@@ -26,12 +32,23 @@ void SamplingProfiler::Start() const {
   double delay_between_samples = 1.0 / num_samples_per_sec_;
   auto task_delay = fml::TimeDelta::FromSecondsF(delay_between_samples);
   UpdateObservatoryThreadName();
+  is_running_ = true;
   SampleRepeatedly(task_delay);
 }
 
+void SamplingProfiler::Stop() {
+  FML_DCHECK(is_running_);
+  auto latch = std::make_unique<fml::AutoResetWaitableEvent>();
+  shutdown_latch_.store(latch.get());
+  latch->Wait();
+  shutdown_latch_.store(nullptr);
+  is_running_ = false;
+}
+
 void SamplingProfiler::SampleRepeatedly(fml::TimeDelta task_delay) const {
   profiler_task_runner_->PostDelayedTask(
-      [profiler = this, task_delay = task_delay, sampler = sampler_]() {
+      [profiler = this, task_delay = task_delay, sampler = sampler_,
+       &shutdown_latch = shutdown_latch_]() {
         // TODO(kaushikiska): consider buffering these every n seconds to
         // avoid spamming the trace buffer.
         const ProfileSample usage = sampler();
@@ -60,7 +77,11 @@ void SamplingProfiler::SampleRepeatedly(fml::TimeDelta task_delay) const {
           TRACE_EVENT_INSTANT1("flutter::profiling", "GpuUsage", "gpu_usage",
                                gpu_usage.c_str());
         }
-        profiler->SampleRepeatedly(task_delay);
+        if (shutdown_latch.load()) {
+          shutdown_latch.load()->Signal();
+        } else {
+          profiler->SampleRepeatedly(task_delay);
+        }
       },
       task_delay);
 }

shell/profiling/sampling_profiler.h

@@ -10,6 +10,7 @@
 #include <optional>
 #include <string>
 
+#include "flutter/fml/synchronization/count_down_latch.h"
 #include "flutter/fml/task_runner.h"
 #include "flutter/fml/trace_event.h"
 
@@ -97,17 +98,23 @@ class SamplingProfiler {
                    Sampler sampler,
                    int num_samples_per_sec);
 
+  ~SamplingProfiler();
+
   /**
    * @brief Starts the SamplingProfiler by triggering `SampleRepeatedly`.
    *
    */
-  void Start() const;
+  void Start();
+
+  void Stop();
 
  private:
   const std::string thread_label_;
   const fml::RefPtr<fml::TaskRunner> profiler_task_runner_;
   const Sampler sampler_;
   const uint32_t num_samples_per_sec_;
+  bool is_running_ = false;
+  std::atomic<fml::AutoResetWaitableEvent*> shutdown_latch_ = nullptr;
 
   void SampleRepeatedly(fml::TimeDelta task_delay) const;
 

shell/profiling/sampling_profiler_unittest.cc

@@ -0,0 +1,64 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/shell/profiling/sampling_profiler.h"
+#include "flutter/fml/message_loop_impl.h"
+#include "flutter/fml/thread.h"
+#include "flutter/testing/testing.h"
+#include "gmock/gmock.h"
+
+using testing::_;
+using testing::Invoke;
+
+namespace fml {
+namespace {
+class MockTaskRunner : public fml::TaskRunner {
+ public:
+  inline static RefPtr<MockTaskRunner> Create() {
+    return AdoptRef(new MockTaskRunner());
+  }
+  MOCK_METHOD1(PostTask, void(const fml::closure& task));
+  MOCK_METHOD2(PostTaskForTime,
+               void(const fml::closure& task, fml::TimePoint target_time));
+  MOCK_METHOD2(PostDelayedTask,
+               void(const fml::closure& task, fml::TimeDelta delay));
+  MOCK_METHOD0(RunsTasksOnCurrentThread, bool());
+  MOCK_METHOD0(GetTaskQueueId, TaskQueueId());
+
+ private:
+  MockTaskRunner() : TaskRunner(fml::RefPtr<MessageLoopImpl>()) {}
+};
+}  // namespace
+}  // namespace fml
+
+namespace flutter {
+
+TEST(SamplingProfilerTest, DeleteAfterStart) {
+  auto thread =
+      std::make_unique<fml::Thread>(flutter::testing::GetCurrentTestName());
+  auto task_runner = fml::MockTaskRunner::Create();
+  std::atomic<int> invoke_count = 0;
+
+  // Ignore calls to PostTask since that would require mocking out calls to
+  // Dart.
+  EXPECT_CALL(*task_runner, PostDelayedTask(_, _))
+      .WillRepeatedly(
+          Invoke([&](const fml::closure& task, fml::TimeDelta delay) {
+            invoke_count.fetch_add(1);
+            thread->GetTaskRunner()->PostTask(task);
+          }));
+
+  {
+    auto profiler = SamplingProfiler(
+        "profiler",
+        /*profiler_task_runner=*/task_runner, [] { return ProfileSample(); },
+        /*num_samples_per_sec=*/1000);
+    profiler.Start();
+  }
+  int invoke_count_at_delete = invoke_count.load();
+  std::this_thread::sleep_for(std::chrono::milliseconds(2));  // nyquist
+  ASSERT_EQ(invoke_count_at_delete, invoke_count.load());
+}
+
+}  // namespace flutter