[Impeller] Fix a race that can abort the process if the Vulkan context is destroyed while pipeline creation tasks are pending (#50883)
The Vulkan pipeline library queues pipeline creation tasks to a ConcurrentTaskRunner. If the ContextVK is destroyed before these tasks execute, then the ConcurrentMessageLoop destructor will delete the pending tasks.
Each task lambda holds a reference to a promise that will be completed with the pipeline. If the task was never run and its promise was never completed, then the promise destructor will complete it with an exception. This will cause a std::abort because Flutter is built without exception support.
This PR wraps the promise in an object that completes it with a default value during destruction if the promise was never given a value.
diff --git a/impeller/base/base_unittests.cc b/impeller/base/base_unittests.cc
index a7c40d8..db2c978 100644
--- a/impeller/base/base_unittests.cc
+++ b/impeller/base/base_unittests.cc
@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "flutter/testing/testing.h"
+#include "impeller/base/promise.h"
#include "impeller/base/strings.h"
#include "impeller/base/thread.h"
@@ -233,5 +234,21 @@
ASSERT_EQ(sum, kThreadCount);
}
+TEST(BaseTest, NoExceptionPromiseValue) {
+ NoExceptionPromise<int> wrapper;
+ std::future future = wrapper.get_future();
+ wrapper.set_value(123);
+ ASSERT_EQ(future.get(), 123);
+}
+
+TEST(BaseTest, NoExceptionPromiseEmpty) {
+ auto wrapper = std::make_shared<NoExceptionPromise<int>>();
+ std::future future = wrapper->get_future();
+
+ // Destroy the empty promise with the future still pending. Verify that the
+ // process does not abort while destructing the promise.
+ wrapper.reset();
+}
+
} // namespace testing
} // namespace impeller
diff --git a/impeller/base/promise.h b/impeller/base/promise.h
index 30d45ae..9f7205e 100644
--- a/impeller/base/promise.h
+++ b/impeller/base/promise.h
@@ -17,6 +17,34 @@
return future;
}
+// Wraps a std::promise and completes the promise with a value during
+// destruction if the promise does not already have a value.
+//
+// By default the std::promise destructor will complete an empty promise with an
+// exception. This will fail because Flutter is built without exception support.
+template <typename T>
+class NoExceptionPromise {
+ public:
+ NoExceptionPromise() = default;
+
+ ~NoExceptionPromise() {
+ if (!value_set_) {
+ promise_.set_value({});
+ }
+ }
+
+ std::future<T> get_future() { return promise_.get_future(); }
+
+ void set_value(const T& value) {
+ promise_.set_value(value);
+ value_set_ = true;
+ }
+
+ private:
+ std::promise<T> promise_;
+ bool value_set_ = false;
+};
+
} // namespace impeller
#endif // FLUTTER_IMPELLER_BASE_PROMISE_H_
diff --git a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc
index 600ef83..130cabd 100644
--- a/impeller/renderer/backend/vulkan/pipeline_library_vk.cc
+++ b/impeller/renderer/backend/vulkan/pipeline_library_vk.cc
@@ -170,7 +170,7 @@
}
auto promise = std::make_shared<
- std::promise<std::shared_ptr<Pipeline<PipelineDescriptor>>>>();
+ NoExceptionPromise<std::shared_ptr<Pipeline<PipelineDescriptor>>>>();
auto pipeline_future =
PipelineFuture<PipelineDescriptor>{descriptor, promise->get_future()};
pipelines_[descriptor] = pipeline_future;