Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit adbd1f4

Browse files
[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.
1 parent b28cfbb commit adbd1f4

File tree

3 files changed

+46
-1
lines changed

3 files changed

+46
-1
lines changed

impeller/base/base_unittests.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
#include "flutter/testing/testing.h"
6+
#include "impeller/base/promise.h"
67
#include "impeller/base/strings.h"
78
#include "impeller/base/thread.h"
89

@@ -233,5 +234,21 @@ TEST(ConditionVariableTest, TestsCriticalSectionAfterWait) {
233234
ASSERT_EQ(sum, kThreadCount);
234235
}
235236

237+
TEST(BaseTest, NoExceptionPromiseValue) {
238+
NoExceptionPromise<int> wrapper;
239+
std::future future = wrapper.get_future();
240+
wrapper.set_value(123);
241+
ASSERT_EQ(future.get(), 123);
242+
}
243+
244+
TEST(BaseTest, NoExceptionPromiseEmpty) {
245+
auto wrapper = std::make_shared<NoExceptionPromise<int>>();
246+
std::future future = wrapper->get_future();
247+
248+
// Destroy the empty promise with the future still pending. Verify that the
249+
// process does not abort while destructing the promise.
250+
wrapper.reset();
251+
}
252+
236253
} // namespace testing
237254
} // namespace impeller

impeller/base/promise.h

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,34 @@ std::future<T> RealizedFuture(T t) {
1717
return future;
1818
}
1919

20+
// Wraps a std::promise and completes the promise with a value during
21+
// destruction if the promise does not already have a value.
22+
//
23+
// By default the std::promise destructor will complete an empty promise with an
24+
// exception. This will fail because Flutter is built without exception support.
25+
template <typename T>
26+
class NoExceptionPromise {
27+
public:
28+
NoExceptionPromise() = default;
29+
30+
~NoExceptionPromise() {
31+
if (!value_set_) {
32+
promise_.set_value({});
33+
}
34+
}
35+
36+
std::future<T> get_future() { return promise_.get_future(); }
37+
38+
void set_value(const T& value) {
39+
promise_.set_value(value);
40+
value_set_ = true;
41+
}
42+
43+
private:
44+
std::promise<T> promise_;
45+
bool value_set_ = false;
46+
};
47+
2048
} // namespace impeller
2149

2250
#endif // FLUTTER_IMPELLER_BASE_PROMISE_H_

impeller/renderer/backend/vulkan/pipeline_library_vk.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ PipelineFuture<PipelineDescriptor> PipelineLibraryVK::GetPipeline(
170170
}
171171

172172
auto promise = std::make_shared<
173-
std::promise<std::shared_ptr<Pipeline<PipelineDescriptor>>>>();
173+
NoExceptionPromise<std::shared_ptr<Pipeline<PipelineDescriptor>>>>();
174174
auto pipeline_future =
175175
PipelineFuture<PipelineDescriptor>{descriptor, promise->get_future()};
176176
pipelines_[descriptor] = pipeline_future;

0 commit comments

Comments
 (0)