diff --git a/engine/src/flutter/shell/platform/embedder/embedder.cc b/engine/src/flutter/shell/platform/embedder/embedder.cc index 1b0caaba289..3d633cd345c 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder.cc +++ b/engine/src/flutter/shell/platform/embedder/embedder.cc @@ -2566,6 +2566,7 @@ FlutterEngineResult FlutterEngineDeinitialize(FLUTTER_API_SYMBOL(FlutterEngine) auto embedder_engine = reinterpret_cast(engine); embedder_engine->NotifyDestroyed(); embedder_engine->CollectShell(); + embedder_engine->CollectThreadHost(); return kSuccess; } @@ -3226,6 +3227,13 @@ FlutterEngineResult FlutterEngineRunTask(FLUTTER_API_SYMBOL(FlutterEngine) return LOG_EMBEDDER_ERROR(kInvalidArguments, "Invalid engine handle."); } + if (!flutter::EmbedderThreadHost::RunnerIsValid( + reinterpret_cast(task->runner))) { + // This task came too late, the embedder has already been destroyed. + // This is not an error, just ignore the task. + return kSuccess; + } + return reinterpret_cast(engine)->RunTask(task) ? kSuccess : LOG_EMBEDDER_ERROR(kInvalidArguments, diff --git a/engine/src/flutter/shell/platform/embedder/embedder_engine.cc b/engine/src/flutter/shell/platform/embedder/embedder_engine.cc index 04d283cf728..ff764224b8f 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_engine.cc +++ b/engine/src/flutter/shell/platform/embedder/embedder_engine.cc @@ -65,6 +65,46 @@ bool EmbedderEngine::CollectShell() { return IsValid(); } +void EmbedderEngine::CollectThreadHost() { + if (!thread_host_) { + return; + } + + // Once the collected, EmbedderThreadHost::RunnerIsValid will return false for + // all runners belonging to this thread host. This must be done with UI task + // runner blocked to prevent possible raciness that could happen when + // destroying the thread host in the middle of UI task runner execution. This + // is not an issue for other runners, because raster task runner should not + // have anything scheduled after engine shutdown and platform task runner is + // where this method is called from. + if (thread_host_->GetTaskRunners().GetUITaskRunner() && + !thread_host_->GetTaskRunners() + .GetUITaskRunner() + ->RunsTasksOnCurrentThread()) { + fml::AutoResetWaitableEvent ui_thread_running; + fml::AutoResetWaitableEvent ui_thread_block; + fml::AutoResetWaitableEvent ui_thread_finished; + + thread_host_->GetTaskRunners().GetUITaskRunner()->PostTask([&] { + ui_thread_running.Signal(); + ui_thread_block.Wait(); + ui_thread_finished.Signal(); + }); + + // Wait until the task is running on the UI thread. + ui_thread_running.Wait(); + thread_host_->InvalidateActiveRunners(); + ui_thread_block.Signal(); + + // Needed to keep ui_thread_block in scope until the UI thread execution + // finishes. + ui_thread_finished.Wait(); + } else { + thread_host_->InvalidateActiveRunners(); + } + thread_host_.reset(); +} + bool EmbedderEngine::RunRootIsolate() { if (!IsValid() || !run_configuration_.IsValid()) { return false; @@ -96,6 +136,7 @@ bool EmbedderEngine::NotifyDestroyed() { } shell_->GetPlatformView()->NotifyDestroyed(); + return true; } @@ -243,7 +284,7 @@ bool EmbedderEngine::RunTask(const FlutterTask* task) { if (task == nullptr) { return false; } - auto result = thread_host_->PostTask(reinterpret_cast(task->runner), + auto result = thread_host_->PostTask(reinterpret_cast(task->runner), task->task); // If the UI and platform threads are separate, the microtask queue is // flushed through MessageLoopTaskQueues observer. diff --git a/engine/src/flutter/shell/platform/embedder/embedder_engine.h b/engine/src/flutter/shell/platform/embedder/embedder_engine.h index a587eeb3eaa..a885237ed13 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_engine.h +++ b/engine/src/flutter/shell/platform/embedder/embedder_engine.h @@ -38,6 +38,8 @@ class EmbedderEngine { bool CollectShell(); + void CollectThreadHost(); + const TaskRunners& GetTaskRunners() const; bool NotifyCreated(); @@ -88,7 +90,7 @@ class EmbedderEngine { Shell& GetShell(); private: - const std::unique_ptr thread_host_; + std::unique_ptr thread_host_; TaskRunners task_runners_; RunConfiguration run_configuration_; std::unique_ptr shell_args_; diff --git a/engine/src/flutter/shell/platform/embedder/embedder_task_runner.cc b/engine/src/flutter/shell/platform/embedder/embedder_task_runner.cc index 00e77954d14..35e2d954721 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_task_runner.cc +++ b/engine/src/flutter/shell/platform/embedder/embedder_task_runner.cc @@ -9,12 +9,15 @@ namespace flutter { +std::atomic_intptr_t EmbedderTaskRunner::next_unique_id_ = 0; + EmbedderTaskRunner::EmbedderTaskRunner(DispatchTable table, size_t embedder_identifier) : TaskRunner(nullptr /* loop implemenation*/), embedder_identifier_(embedder_identifier), dispatch_table_(std::move(table)), - placeholder_id_(fml::TaskQueueId(fml::TaskQueueId::kInvalid)) { + placeholder_id_(fml::TaskQueueId(fml::TaskQueueId::kInvalid)), + unique_id_(next_unique_id_++) { FML_DCHECK(dispatch_table_.post_task_callback); FML_DCHECK(dispatch_table_.runs_task_on_current_thread_callback); FML_DCHECK(dispatch_table_.destruction_callback); diff --git a/engine/src/flutter/shell/platform/embedder/embedder_task_runner.h b/engine/src/flutter/shell/platform/embedder/embedder_task_runner.h index 4f7b47bd76e..ba35500a82d 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_task_runner.h +++ b/engine/src/flutter/shell/platform/embedder/embedder_task_runner.h @@ -75,6 +75,8 @@ class EmbedderTaskRunner final : public fml::TaskRunner { bool PostTask(uint64_t baton); + intptr_t unique_id() const { return unique_id_; } + private: const size_t embedder_identifier_; DispatchTable dispatch_table_; @@ -82,6 +84,9 @@ class EmbedderTaskRunner final : public fml::TaskRunner { uint64_t last_baton_ = 0; std::unordered_map pending_tasks_; fml::TaskQueueId placeholder_id_; + intptr_t unique_id_; + + static std::atomic_intptr_t next_unique_id_; // |fml::TaskRunner| void PostTask(const fml::closure& task) override; diff --git a/engine/src/flutter/shell/platform/embedder/embedder_thread_host.cc b/engine/src/flutter/shell/platform/embedder/embedder_thread_host.cc index e84e2ad21d7..0532c16eecf 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_thread_host.cc +++ b/engine/src/flutter/shell/platform/embedder/embedder_thread_host.cc @@ -13,6 +13,9 @@ namespace flutter { +std::set EmbedderThreadHost::active_runners_; +std::mutex EmbedderThreadHost::active_runners_mutex_; + //------------------------------------------------------------------------------ /// @brief Attempts to create a task runner from an embedder task runner /// description. The first boolean in the pair indicate whether the @@ -67,7 +70,7 @@ CreateEmbedderTaskRunner(const FlutterTaskRunnerDescription* description) { fml::TimePoint target_time) -> void { FlutterTask task = { // runner - reinterpret_cast(task_runner), + reinterpret_cast(task_runner->unique_id()), // task task_baton, }; @@ -306,13 +309,27 @@ EmbedderThreadHost::EmbedderThreadHost( const flutter::TaskRunners& runners, const std::set>& embedder_task_runners) : host_(std::move(host)), runners_(runners) { + std::lock_guard guard(active_runners_mutex_); for (const auto& runner : embedder_task_runners) { - runners_map_[reinterpret_cast(runner.get())] = runner; + runners_map_[runner->unique_id()] = runner; + active_runners_.insert(runner->unique_id()); } } EmbedderThreadHost::~EmbedderThreadHost() = default; +void EmbedderThreadHost::InvalidateActiveRunners() { + std::lock_guard guard(active_runners_mutex_); + for (const auto& runner : runners_map_) { + active_runners_.erase(runner.first); + } +} + +bool EmbedderThreadHost::RunnerIsValid(intptr_t runner) { + std::lock_guard guard(active_runners_mutex_); + return active_runners_.find(runner) != active_runners_.end(); +} + bool EmbedderThreadHost::IsValid() const { return runners_.IsValid(); } @@ -321,7 +338,7 @@ const flutter::TaskRunners& EmbedderThreadHost::GetTaskRunners() const { return runners_; } -bool EmbedderThreadHost::PostTask(int64_t runner, uint64_t task) const { +bool EmbedderThreadHost::PostTask(intptr_t runner, uint64_t task) const { auto found = runners_map_.find(runner); if (found == runners_map_.end()) { return false; diff --git a/engine/src/flutter/shell/platform/embedder/embedder_thread_host.h b/engine/src/flutter/shell/platform/embedder/embedder_thread_host.h index 054812892e8..6213383c643 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_thread_host.h +++ b/engine/src/flutter/shell/platform/embedder/embedder_thread_host.h @@ -36,12 +36,19 @@ class EmbedderThreadHost { const flutter::TaskRunners& GetTaskRunners() const; - bool PostTask(int64_t runner, uint64_t task) const; + bool PostTask(intptr_t runner, uint64_t task) const; + + static bool RunnerIsValid(intptr_t runner); + + void InvalidateActiveRunners(); private: ThreadHost host_; flutter::TaskRunners runners_; - std::map> runners_map_; + std::map> runners_map_; + + static std::set active_runners_; + static std::mutex active_runners_mutex_; static std::unique_ptr CreateEmbedderManagedThreadHost( const FlutterCustomTaskRunners* custom_task_runners, diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc index aa840afb84c..6f2cb0f7031 100644 --- a/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc +++ b/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc @@ -269,6 +269,73 @@ TEST_F(EmbedderTest, CanSpecifyCustomUITaskRunner) { kill_latch.Wait(); } +TEST_F(EmbedderTest, IgnoresStaleTasks) { + auto& context = GetEmbedderContext(); + auto ui_task_runner = CreateNewThread("test_ui_thread"); + auto platform_task_runner = CreateNewThread("test_platform_thread"); + static std::mutex engine_mutex; + UniqueEngine engine; + FlutterEngine engine_ptr; + + EmbedderTestTaskRunner test_ui_task_runner( + ui_task_runner, [&](FlutterTask task) { + // The check for engine.is_valid() is intentionally absent here. + // FlutterEngineRunTask must be able to detect and ignore stale tasks + // without crashing even if the engine pointer is not null. + // Because the engine is destroyed on platform thread, + // relying solely on engine.is_valid() in UI thread is not safe. + FlutterEngineRunTask(engine_ptr, &task); + }); + EmbedderTestTaskRunner test_platform_task_runner( + platform_task_runner, [&](FlutterTask task) { + std::scoped_lock lock(engine_mutex); + if (!engine.is_valid()) { + return; + } + FlutterEngineRunTask(engine.get(), &task); + }); + + fml::AutoResetWaitableEvent init_latch; + + platform_task_runner->PostTask([&]() { + EmbedderConfigBuilder builder(context); + const auto ui_task_runner_description = + test_ui_task_runner.GetFlutterTaskRunnerDescription(); + const auto platform_task_runner_description = + test_platform_task_runner.GetFlutterTaskRunnerDescription(); + builder.SetUITaskRunner(&ui_task_runner_description); + builder.SetPlatformTaskRunner(&platform_task_runner_description); + { + std::scoped_lock lock(engine_mutex); + engine = builder.InitializeEngine(); + } + init_latch.Signal(); + }); + + init_latch.Wait(); + engine_ptr = engine.get(); + + auto flutter_engine = reinterpret_cast(engine.get()); + + // Schedule task on UI thread that will likely run after the engine has shut + // down. + flutter_engine->GetTaskRunners().GetUITaskRunner()->PostDelayedTask( + []() {}, fml::TimeDelta::FromMilliseconds(50)); + + fml::AutoResetWaitableEvent kill_latch; + platform_task_runner->PostTask([&] { + engine.reset(); + platform_task_runner->PostTask([&kill_latch] { kill_latch.Signal(); }); + }); + kill_latch.Wait(); + + // Ensure that the schedule task indeed runs. + kill_latch.Reset(); + ui_task_runner->PostDelayedTask([&]() { kill_latch.Signal(); }, + fml::TimeDelta::FromMilliseconds(50)); + kill_latch.Wait(); +} + TEST_F(EmbedderTest, MergedPlatformUIThread) { auto& context = GetEmbedderContext(); auto task_runner = CreateNewThread("test_thread");