[Impeller] Fix thread leak in ResourceManagerVK.

impeller/renderer/backend/vulkan/resource_manager_vk.cc

@@ -11,22 +11,26 @@
 namespace impeller {
 
 std::shared_ptr<ResourceManagerVK> ResourceManagerVK::Create() {
-  auto manager = std::shared_ptr<ResourceManagerVK>(new ResourceManagerVK());
-  manager->waiter_ = std::thread([manager]() { manager->Start(); });
-  manager->waiter_.detach();
-  return manager;
+  // It will be tempting to refactor this to create the waiter thread in the
+  // static method instead of the constructor. However, that causes the
+  // destructor never to be called, and the thread never terminates!
+  //
+  // See https://github.com/flutter/flutter/issues/134482.
+  return std::shared_ptr<ResourceManagerVK>(new ResourceManagerVK());
 }
 
-ResourceManagerVK::ResourceManagerVK() {}
+ResourceManagerVK::ResourceManagerVK() : waiter_([&]() { Start(); }) {}
 
 ResourceManagerVK::~ResourceManagerVK() {
   Terminate();
   waiter_.join();
 }
 
 void ResourceManagerVK::Start() {
-  // This thread should not be started more than once.
-  FML_DCHECK(!should_exit_);
+  // It's possible for Start() to be called when terminating:
+  // { ResourceManagerVK::Create(); }
+  //
+  // ... so no FML_DCHECK here.
 
   fml::Thread::SetCurrentThreadName(
       fml::Thread::ThreadConfig{"io.flutter.impeller.resource_manager"});

impeller/renderer/backend/vulkan/resource_manager_vk_unittests.cc

@@ -6,6 +6,7 @@
 #include <functional>
 #include <memory>
 #include <utility>
+#include "fml/logging.h"
 #include "fml/synchronization/waitable_event.h"
 #include "gtest/gtest.h"
 #include "impeller/renderer/backend/vulkan/resource_manager_vk.h"
@@ -58,5 +59,21 @@ TEST(ResourceManagerVKTest, ReclaimMovesAResourceAndDestroysIt) {
   waiter.Wait();
 }
 
+// Regression test for https://github.com/flutter/flutter/issues/134482.
+TEST(ResourceManagerVKTest, TerminatesWhenOutOfScope) {
+  // Originally, this shared_ptr was never destroyed, and the thread never
+  // terminated. This test ensures that the thread terminates when the
+  // ResourceManagerVK is out of scope.
+  std::weak_ptr<ResourceManagerVK> manager;
+
+  {
+    auto shared = ResourceManagerVK::Create();
+    manager = shared;
+  }
+
+  // The thread should have terminated.
+  EXPECT_EQ(manager.lock(), nullptr);
+}
+
 }  // namespace testing
 }  // namespace impeller