Introduce TaskRunnerChecker, TaskRunnerAffineWeakPtr (#17649)

ci/licenses_golden/licenses_flutter

@@ -153,6 +153,9 @@ FILE: ../../../flutter/fml/memory/ref_counted_internal.h
 FILE: ../../../flutter/fml/memory/ref_counted_unittest.cc
 FILE: ../../../flutter/fml/memory/ref_ptr.h
 FILE: ../../../flutter/fml/memory/ref_ptr_internal.h
+FILE: ../../../flutter/fml/memory/task_runner_checker.cc
+FILE: ../../../flutter/fml/memory/task_runner_checker.h
+FILE: ../../../flutter/fml/memory/task_runner_checker_unittest.cc
 FILE: ../../../flutter/fml/memory/thread_checker.h
 FILE: ../../../flutter/fml/memory/weak_ptr.h
 FILE: ../../../flutter/fml/memory/weak_ptr_internal.cc

fml/BUILD.gn

@@ -43,6 +43,8 @@ source_set("fml") {
     "memory/ref_counted_internal.h",
     "memory/ref_ptr.h",
     "memory/ref_ptr_internal.h",
+    "memory/task_runner_checker.cc",
+    "memory/task_runner_checker.h",
     "memory/thread_checker.h",
     "memory/weak_ptr.h",
     "memory/weak_ptr_internal.cc",
@@ -237,6 +239,7 @@ executable("fml_unittests") {
     "file_unittest.cc",
     "hash_combine_unittests.cc",
     "memory/ref_counted_unittest.cc",
+    "memory/task_runner_checker_unittest.cc",
     "memory/weak_ptr_unittest.cc",
     "message_loop_task_queues_merge_unmerge_unittests.cc",
     "message_loop_task_queues_unittests.cc",

fml/memory/task_runner_checker.cc

+// 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/fml/memory/task_runner_checker.h"
+
+namespace fml {
+
+TaskRunnerChecker::TaskRunnerChecker()
+    : initialized_queue_id_(InitTaskQueueId()){};
+
+TaskRunnerChecker::~TaskRunnerChecker() = default;
+
+bool TaskRunnerChecker::RunsOnCreationTaskRunner() const {
+  FML_CHECK(fml::MessageLoop::IsInitializedForCurrentThread());
+
+  const auto current_queue_id = MessageLoop::GetCurrentTaskQueueId();
+
+  if (current_queue_id == initialized_queue_id_) {
+    return true;
+  }
+
+  auto queues = MessageLoopTaskQueues::GetInstance();
+  if (queues->Owns(current_queue_id, initialized_queue_id_)) {
+    return true;
+  }
+  if (queues->Owns(initialized_queue_id_, current_queue_id)) {
+    return true;
+  }
+  return false;
+};
+
+TaskQueueId TaskRunnerChecker::InitTaskQueueId() {
+  MessageLoop::EnsureInitializedForCurrentThread();
+  return MessageLoop::GetCurrentTaskQueueId();
+};
+
+}  // namespace fml

fml/memory/task_runner_checker.h

+// 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.
+
+#ifndef FLUTTER_FML_MEMORY_TASK_RUNNER_CHECKER_H_
+#define FLUTTER_FML_MEMORY_TASK_RUNNER_CHECKER_H_
+
+#include "flutter/fml/message_loop.h"
+#include "flutter/fml/task_runner.h"
+
+namespace fml {
+
+class TaskRunnerChecker final {
+ public:
+  TaskRunnerChecker();
+
+  ~TaskRunnerChecker();
+
+  bool RunsOnCreationTaskRunner() const;
+
+ private:
+  TaskQueueId initialized_queue_id_;
+
+  TaskQueueId InitTaskQueueId();
+};
+
+#if !defined(NDEBUG)
+#define FML_DECLARE_TASK_RUNNER_CHECKER(c) fml::TaskRunnerChecker c
+#define FML_DCHECK_TASK_RUNNER_IS_CURRENT(c) \
+  FML_DCHECK((c).RunsOnCreationTaskRunner())
+#else
+#define FML_DECLARE_TASK_RUNNER_CHECKER(c)
+#define FML_DCHECK_TASK_RUNNER_IS_CURRENT(c) ((void)0)
+#endif
+
+}  // namespace fml
+
+#endif  // FLUTTER_FML_MEMORY_THREAD_CHECKER_H_

fml/memory/task_runner_checker_unittest.cc

+// 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.
+
+#define FML_USED_ON_EMBEDDER
+
+#include <thread>
+
+#include "flutter/fml/memory/task_runner_checker.h"
+#include "flutter/fml/message_loop.h"
+#include "flutter/fml/synchronization/count_down_latch.h"
+#include "flutter/fml/synchronization/waitable_event.h"
+
+#include <gtest/gtest.h>
+
+namespace fml {
+namespace testing {
+
+TEST(TaskRunnerCheckerTests, RunsOnCurrentTaskRunner) {
+  TaskRunnerChecker checker;
+  EXPECT_EQ(checker.RunsOnCreationTaskRunner(), true);
+}
+
+TEST(TaskRunnerCheckerTests, FailsTheCheckIfOnDifferentTaskRunner) {
+  TaskRunnerChecker checker;
+  EXPECT_EQ(checker.RunsOnCreationTaskRunner(), true);
+  fml::MessageLoop* loop = nullptr;
+  fml::AutoResetWaitableEvent latch;
+  std::thread anotherThread([&]() {
+    fml::MessageLoop::EnsureInitializedForCurrentThread();
+    loop = &fml::MessageLoop::GetCurrent();
+    loop->GetTaskRunner()->PostTask([&]() {
+      EXPECT_EQ(checker.RunsOnCreationTaskRunner(), false);
+      latch.Signal();
+    });
+    loop->Run();
+  });
+  latch.Wait();
+  loop->Terminate();
+  anotherThread.join();
+  EXPECT_EQ(checker.RunsOnCreationTaskRunner(), true);
+}
+
+}  // namespace testing
+}  // namespace fml

fml/memory/weak_ptr.h

@@ -12,6 +12,7 @@
 
 #include "flutter/fml/logging.h"
 #include "flutter/fml/memory/ref_counted.h"
+#include "flutter/fml/memory/task_runner_checker.h"
 #include "flutter/fml/memory/thread_checker.h"
 #include "flutter/fml/memory/weak_ptr_internal.h"
 
@@ -21,6 +22,10 @@ struct DebugThreadChecker {
   FML_DECLARE_THREAD_CHECKER(checker);
 };
 
+struct DebugTaskRunnerChecker {
+  FML_DECLARE_TASK_RUNNER_CHECKER(checker);
+};
+
 // Forward declaration, so |WeakPtr<T>| can friend it.
 template <typename T>
 class WeakPtrFactory;
@@ -43,7 +48,7 @@ class WeakPtr {
   WeakPtr() : ptr_(nullptr) {}
 
   // Copy constructor.
-  WeakPtr(const WeakPtr<T>& r) = default;
+  explicit WeakPtr(const WeakPtr<T>& r) = default;
 
   template <typename U>
   WeakPtr(const WeakPtr<U>& r)
@@ -58,7 +63,7 @@ class WeakPtr {
         flag_(std::move(r.flag_)),
         checker_(r.checker_) {}
 
-  ~WeakPtr() = default;
+  virtual ~WeakPtr() = default;
 
   // The following methods are thread-friendly, in the sense that they may be
   // called subject to additional synchronization.
@@ -75,12 +80,12 @@ class WeakPtr {
   // "originating" |WeakPtrFactory|.
 
   explicit operator bool() const {
-    FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker);
+    CheckThreadSafety();
     return flag_ && flag_->is_valid();
   }
 
   T* get() const {
-    FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker);
+    CheckThreadSafety();
     return *this ? ptr_ : nullptr;
   }
 
@@ -97,17 +102,28 @@ class WeakPtr {
   }
 
   T& operator*() const {
-    FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker);
+    CheckThreadSafety();
     FML_DCHECK(*this);
     return *get();
   }
 
   T* operator->() const {
-    FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker);
+    CheckThreadSafety();
     FML_DCHECK(*this);
     return get();
   }
 
+ protected:
+  explicit WeakPtr(T* ptr, fml::RefPtr<fml::internal::WeakPtrFlag>&& flag)
+      : ptr_(ptr), flag_(std::move(flag)) {}
+
+  T* ptr_;
+  fml::RefPtr<fml::internal::WeakPtrFlag> flag_;
+
+  virtual void CheckThreadSafety() const {
+    FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker);
+  }
+
  private:
   template <typename U>
   friend class WeakPtr;
@@ -119,13 +135,58 @@ class WeakPtr {
                    DebugThreadChecker checker)
       : ptr_(ptr), flag_(std::move(flag)), checker_(checker) {}
 
-  T* ptr_;
-  fml::RefPtr<fml::internal::WeakPtrFlag> flag_;
   DebugThreadChecker checker_;
 
   // Copy/move construction/assignment supported.
 };
 
+// A weak pointer that can be used in different threads as long as
+// the threads are belong to the same |TaskRunner|.
+//
+// It is still not in general thread safe as |WeakPtr|.
+template <typename T>
+class TaskRunnerAffineWeakPtr : public WeakPtr<T> {
+ public:
+  TaskRunnerAffineWeakPtr() : WeakPtr<T>() {}
+
+  TaskRunnerAffineWeakPtr(const TaskRunnerAffineWeakPtr<T>& r) = default;
+
+  template <typename U>
+  TaskRunnerAffineWeakPtr(const TaskRunnerAffineWeakPtr<U>& r)
+      : WeakPtr<T>(static_cast<T*>(r.ptr_), r.flag_), checker_(r.checker_) {}
+
+  TaskRunnerAffineWeakPtr(TaskRunnerAffineWeakPtr<T>&& r) = default;
+
+  template <typename U>
+  TaskRunnerAffineWeakPtr(TaskRunnerAffineWeakPtr<U>&& r)
+      : WeakPtr<T>(static_cast<T*>(r.ptr_), std::move(r.flag_)),
+        checker_(r.checker_) {}
+
+  ~TaskRunnerAffineWeakPtr() = default;
+
+  TaskRunnerAffineWeakPtr<T>& operator=(const TaskRunnerAffineWeakPtr<T>& r) =
+      default;
+
+  TaskRunnerAffineWeakPtr<T>& operator=(TaskRunnerAffineWeakPtr<T>&& r) =
+      default;
+
+ protected:
+  void CheckThreadSafety() const override {
+    FML_DCHECK_TASK_RUNNER_IS_CURRENT(checker_.checker);
+  }
+
+ private:
+  friend class WeakPtrFactory<T>;
+
+  explicit TaskRunnerAffineWeakPtr(
+      T* ptr,
+      fml::RefPtr<fml::internal::WeakPtrFlag>&& flag,
+      DebugTaskRunnerChecker checker)
+      : WeakPtr<T>(ptr, std::move(flag)), checker_(checker) {}
+
+  DebugTaskRunnerChecker checker_;
+};
+
 // Class that produces (valid) |WeakPtr<T>|s. Typically, this is used as a
 // member variable of |T| (preferably the last one -- see below), and |T|'s
 // methods control how weak pointers to it are vended. This class is not
@@ -177,7 +238,7 @@ class WeakPtrFactory {
   }
 
   ~WeakPtrFactory() {
-    FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker);
+    CheckThreadSafety();
     flag_->Invalidate();
   }
 
@@ -187,12 +248,26 @@ class WeakPtrFactory {
     return WeakPtr<T>(ptr_, flag_.Clone(), checker_);
   }
 
- private:
+  // Gets a new weak pointer, which will be valid until either
+  // |InvalidateWeakPtrs()| is called or this object is destroyed.
+  TaskRunnerAffineWeakPtr<T> GetTaskRunnerAffineWeakPtr() const {
+    return TaskRunnerAffineWeakPtr<T>(ptr_, flag_.Clone(),
+                                      task_runner_checker_);
+  }
+
+ protected:
   // Note: See weak_ptr_internal.h for an explanation of why we store the
   // pointer here, instead of in the "flag".
   T* const ptr_;
   fml::RefPtr<fml::internal::WeakPtrFlag> flag_;
+
+  void CheckThreadSafety() const {
+    FML_DCHECK_CREATION_THREAD_IS_CURRENT(checker_.checker);
+  }
+
+ private:
   DebugThreadChecker checker_;
+  DebugTaskRunnerChecker task_runner_checker_;
 
   FML_DISALLOW_COPY_AND_ASSIGN(WeakPtrFactory);
 };

fml/memory/weak_ptr_unittest.cc

@@ -2,10 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#define FML_USED_ON_EMBEDDER
+
 #include "flutter/fml/memory/weak_ptr.h"
 
+#include <thread>
 #include <utility>
 
+#include "flutter/fml/message_loop.h"
+#include "flutter/fml/raster_thread_merger.h"
+#include "flutter/fml/synchronization/count_down_latch.h"
 #include "gtest/gtest.h"
 
 namespace fml {
@@ -162,5 +168,66 @@ TEST(WeakPtrTest, UpcastMoveAssignment) {
   EXPECT_EQ(&data, ptr2.get());
 }
 
+TEST(TaskRunnerAffineWeakPtrTest, ShouldNotCrashIfRunningOnTheSameTaskRunner) {
+  fml::MessageLoop* loop1 = nullptr;
+  fml::AutoResetWaitableEvent latch1;
+  fml::AutoResetWaitableEvent term1;
+  std::thread thread1([&loop1, &latch1, &term1]() {
+    fml::MessageLoop::EnsureInitializedForCurrentThread();
+    loop1 = &fml::MessageLoop::GetCurrent();
+    latch1.Signal();
+    term1.Wait();
+  });
+
+  fml::MessageLoop* loop2 = nullptr;
+  fml::AutoResetWaitableEvent latch2;
+  fml::AutoResetWaitableEvent term2;
+  fml::AutoResetWaitableEvent loop2_task_finish_latch;
+  fml::AutoResetWaitableEvent loop2_task_start_latch;
+  std::thread thread2([&loop2, &latch2, &term2, &loop2_task_finish_latch,
+                       &loop2_task_start_latch]() {
+    fml::MessageLoop::EnsureInitializedForCurrentThread();
+    int data = 0;
+    WeakPtrFactory<int> factory(&data);
+    loop2 = &fml::MessageLoop::GetCurrent();
+
+    loop2->GetTaskRunner()->PostTask([&]() {
+      latch2.Signal();
+      loop2_task_start_latch.Wait();
+      TaskRunnerAffineWeakPtr<int> ptr = factory.GetTaskRunnerAffineWeakPtr();
+      EXPECT_EQ(*ptr, data);
+      loop2_task_finish_latch.Signal();
+    });
+    loop2->Run();
+    term2.Wait();
+  });
+
+  latch1.Wait();
+  latch2.Wait();
+  fml::TaskQueueId qid1 = loop1->GetTaskRunner()->GetTaskQueueId();
+  fml::TaskQueueId qid2 = loop2->GetTaskRunner()->GetTaskQueueId();
+  const auto raster_thread_merger_ =
+      fml::MakeRefCounted<fml::RasterThreadMerger>(qid1, qid2);
+  const int kNumFramesMerged = 5;
+
+  raster_thread_merger_->MergeWithLease(kNumFramesMerged);
+
+  loop2_task_start_latch.Signal();
+  loop2_task_finish_latch.Wait();
+
+  for (int i = 0; i < kNumFramesMerged; i++) {
+    ASSERT_TRUE(raster_thread_merger_->IsMerged());
+    raster_thread_merger_->DecrementLease();
+  }
+
+  ASSERT_FALSE(raster_thread_merger_->IsMerged());
+  loop2->Terminate();
+
+  term1.Signal();
+  term2.Signal();
+  thread1.join();
+  thread2.join();
+}
+
 }  // namespace
 }  // namespace fml

shell/common/rasterizer.cc

@@ -53,8 +53,8 @@ Rasterizer::Rasterizer(
 
 Rasterizer::~Rasterizer() = default;
 
-fml::WeakPtr<Rasterizer> Rasterizer::GetWeakPtr() const {
-  return weak_factory_.GetWeakPtr();
+fml::TaskRunnerAffineWeakPtr<Rasterizer> Rasterizer::GetWeakPtr() const {
+  return weak_factory_.GetTaskRunnerAffineWeakPtr();
 }
 
 fml::WeakPtr<SnapshotDelegate> Rasterizer::GetSnapshotDelegate() const {
@@ -155,7 +155,7 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
   switch (consume_result) {
     case PipelineConsumeResult::MoreAvailable: {
       task_runners_.GetRasterTaskRunner()->PostTask(
-          [weak_this = weak_factory_.GetWeakPtr(), pipeline]() {
+          [weak_this = weak_factory_.GetTaskRunnerAffineWeakPtr(), pipeline]() {
             if (weak_this) {
               weak_this->Draw(pipeline);
             }

shell/common/rasterizer.h

@@ -184,7 +184,7 @@ class Rasterizer final : public SnapshotDelegate {
   ///
   /// @return     The weak pointer to the rasterizer.
   ///
-  fml::WeakPtr<Rasterizer> GetWeakPtr() const;
+  fml::TaskRunnerAffineWeakPtr<Rasterizer> GetWeakPtr() const;
 
   fml::WeakPtr<SnapshotDelegate> GetSnapshotDelegate() const;
 

shell/common/shell.cc

@@ -575,7 +575,7 @@ const TaskRunners& Shell::GetTaskRunners() const {
   return task_runners_;
 }
 
-fml::WeakPtr<Rasterizer> Shell::GetRasterizer() const {
+fml::TaskRunnerAffineWeakPtr<Rasterizer> Shell::GetRasterizer() const {
   FML_DCHECK(is_setup_);
   return weak_rasterizer_;
 }
@@ -1149,7 +1149,7 @@ void Shell::OnFrameRasterized(const FrameTiming& timing) {
     // never be reported until the next animation starts.
     frame_timings_report_scheduled_ = true;
     task_runners_.GetRasterTaskRunner()->PostDelayedTask(
-        [self = weak_factory_gpu_->GetWeakPtr()]() {
+        [self = weak_factory_gpu_->GetTaskRunnerAffineWeakPtr()]() {
           if (!self.get()) {
             return;
           }

shell/common/shell.h

@@ -255,7 +255,7 @@ class Shell final : public PlatformView::Delegate,
   ///
   /// @return     A weak pointer to the rasterizer.
   ///
-  fml::WeakPtr<Rasterizer> GetRasterizer() const;
+  fml::TaskRunnerAffineWeakPtr<Rasterizer> GetRasterizer() const;
 
   //------------------------------------------------------------------------------
   /// @brief      Engines may only be accessed on the UI thread. This method is
@@ -378,8 +378,9 @@ class Shell final : public PlatformView::Delegate,
   std::unique_ptr<ShellIOManager> io_manager_;   // on IO task runner
   std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch_;
 
-  fml::WeakPtr<Engine> weak_engine_;          // to be shared across threads
-  fml::WeakPtr<Rasterizer> weak_rasterizer_;  // to be shared across threads
+  fml::WeakPtr<Engine> weak_engine_;  // to be shared across threads
+  fml::TaskRunnerAffineWeakPtr<Rasterizer>
+      weak_rasterizer_;  // to be shared across threads
   fml::WeakPtr<PlatformView>
       weak_platform_view_;  // to be shared across threads
 

shell/gpu/gpu_surface_gl.cc

@@ -257,8 +257,8 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceGL::AcquireFrame(const SkISize& size) {
   surface->getCanvas()->setMatrix(root_surface_transformation);
 
   SurfaceFrame::SubmitCallback submit_callback =
-      [weak = weak_factory_.GetWeakPtr()](const SurfaceFrame& surface_frame,
-                                          SkCanvas* canvas) {
+      [weak = weak_factory_.GetTaskRunnerAffineWeakPtr()](
+          const SurfaceFrame& surface_frame, SkCanvas* canvas) {
         return weak ? weak->PresentSurface(canvas) : false;
       };
 

shell/gpu/gpu_surface_software.cc

@@ -57,8 +57,8 @@ std::unique_ptr<SurfaceFrame> GPUSurfaceSoftware::AcquireFrame(
   canvas->resetMatrix();
 
   SurfaceFrame::SubmitCallback on_submit =
-      [self = weak_factory_.GetWeakPtr()](const SurfaceFrame& surface_frame,
-                                          SkCanvas* canvas) -> bool {
+      [self = weak_factory_.GetTaskRunnerAffineWeakPtr()](
+          const SurfaceFrame& surface_frame, SkCanvas* canvas) -> bool {
     // If the surface itself went away, there is nothing more to do.
     if (!self || !self->IsValid() || canvas == nullptr) {
       return false;