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

Commit 4ecfa62

Browse files
jason-simmonsjason-simmons
authored
Hold a reference to the Skia unref queue in UIDartState (#13239)
Obtaining the SkiaUnrefQueue through the IOManager is unsafe because UIDartState has a weak pointer to the IOManager that can not be dereferenced on the UI thread.
1 parent 4f85010 commit 4ecfa62

File tree

11 files changed

+50
-24
lines changed

11 files changed

+50
-24
lines changed

‎lib/ui/ui_dart_state.cc‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ UIDartState::UIDartState(
1818
TaskObserverAdd add_callback,
1919
TaskObserverRemove remove_callback,
2020
fml::WeakPtr<IOManager> io_manager,
21+
fml::RefPtr<SkiaUnrefQueue> skia_unref_queue,
2122
fml::WeakPtr<ImageDecoder> image_decoder,
2223
std::string advisory_script_uri,
2324
std::string advisory_script_entrypoint,
@@ -28,6 +29,7 @@ UIDartState::UIDartState(
2829
add_callback_(std::move(add_callback)),
2930
remove_callback_(std::move(remove_callback)),
3031
io_manager_(std::move(io_manager)),
32+
skia_unref_queue_(std::move(skia_unref_queue)),
3133
image_decoder_(std::move(image_decoder)),
3234
advisory_script_uri_(std::move(advisory_script_uri)),
3335
advisory_script_entrypoint_(std::move(advisory_script_entrypoint)),
@@ -83,16 +85,7 @@ fml::WeakPtr<IOManager> UIDartState::GetIOManager() const {
8385
}
8486

8587
fml::RefPtr<flutter::SkiaUnrefQueue> UIDartState::GetSkiaUnrefQueue() const {
86-
// TODO(gw280): The WeakPtr here asserts that we are derefing it on the
87-
// same thread as it was created on. As we can't guarantee that currently
88-
// we're being called from the IO thread (construction thread), we need
89-
// to use getUnsafe() here to avoid failing the assertion.
90-
//
91-
// https://github.com/flutter/flutter/issues/42946
92-
if (!io_manager_.getUnsafe()) {
93-
return nullptr;
94-
}
95-
return io_manager_.getUnsafe()->GetSkiaUnrefQueue();
88+
return skia_unref_queue_;
9689
}
9790

9891
void UIDartState::ScheduleMicrotask(Dart_Handle closure) {

‎lib/ui/ui_dart_state.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ class UIDartState : public tonic::DartState {
7777
TaskObserverAdd add_callback,
7878
TaskObserverRemove remove_callback,
7979
fml::WeakPtr<IOManager> io_manager,
80+
fml::RefPtr<SkiaUnrefQueue> skia_unref_queue,
8081
fml::WeakPtr<ImageDecoder> image_decoder,
8182
std::string advisory_script_uri,
8283
std::string advisory_script_entrypoint,
@@ -99,6 +100,7 @@ class UIDartState : public tonic::DartState {
99100
const TaskObserverAdd add_callback_;
100101
const TaskObserverRemove remove_callback_;
101102
fml::WeakPtr<IOManager> io_manager_;
103+
fml::RefPtr<SkiaUnrefQueue> skia_unref_queue_;
102104
fml::WeakPtr<ImageDecoder> image_decoder_;
103105
const std::string advisory_script_uri_;
104106
const std::string advisory_script_entrypoint_;

‎runtime/dart_isolate.cc‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
3636
TaskRunners task_runners,
3737
std::unique_ptr<Window> window,
3838
fml::WeakPtr<IOManager> io_manager,
39+
fml::RefPtr<SkiaUnrefQueue> unref_queue,
3940
fml::WeakPtr<ImageDecoder> image_decoder,
4041
std::string advisory_script_uri,
4142
std::string advisory_script_entrypoint,
@@ -61,6 +62,7 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
6162
std::move(shared_snapshot), // shared snapshot
6263
task_runners, // task runners
6364
std::move(io_manager), // IO manager
65+
std::move(unref_queue), // Skia unref queue
6466
std::move(image_decoder), // Image Decoder
6567
advisory_script_uri, // advisory URI
6668
advisory_script_entrypoint, // advisory entrypoint
@@ -105,6 +107,7 @@ DartIsolate::DartIsolate(const Settings& settings,
105107
fml::RefPtr<const DartSnapshot> shared_snapshot,
106108
TaskRunners task_runners,
107109
fml::WeakPtr<IOManager> io_manager,
110+
fml::RefPtr<SkiaUnrefQueue> unref_queue,
108111
fml::WeakPtr<ImageDecoder> image_decoder,
109112
std::string advisory_script_uri,
110113
std::string advisory_script_entrypoint,
@@ -115,6 +118,7 @@ DartIsolate::DartIsolate(const Settings& settings,
115118
settings.task_observer_add,
116119
settings.task_observer_remove,
117120
std::move(io_manager),
121+
std::move(unref_queue),
118122
std::move(image_decoder),
119123
advisory_script_uri,
120124
advisory_script_entrypoint,
@@ -596,6 +600,7 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate(
596600
null_task_runners, // task runners
597601
nullptr, // window
598602
{}, // IO Manager
603+
{}, // Skia unref queue
599604
{}, // Image Decoder
600605
DART_VM_SERVICE_ISOLATE_NAME, // script uri
601606
DART_VM_SERVICE_ISOLATE_NAME, // script entrypoint
@@ -708,7 +713,8 @@ DartIsolate::CreateDartVMAndEmbedderObjectPair(
708713
(*raw_embedder_isolate)->GetSharedSnapshot(), // shared_snapshot
709714
null_task_runners, // task_runners
710715
fml::WeakPtr<IOManager>{}, // io_manager
711-
fml::WeakPtr<ImageDecoder>{}, // io_manager
716+
fml::RefPtr<SkiaUnrefQueue>{}, // unref_queue
717+
fml::WeakPtr<ImageDecoder>{}, // image_decoder
712718
advisory_script_uri, // advisory_script_uri
713719
advisory_script_entrypoint, // advisory_script_entrypoint
714720
(*raw_embedder_isolate)->child_isolate_preparer_, // preparer

‎runtime/dart_isolate.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ class DartIsolate : public UIDartState {
156156
/// @param[in] window The weak pointer to the window
157157
/// associated with this root isolate.
158158
/// @param[in] io_manager The i/o manager.
159+
/// @param[in] unref_queue The Skia unref queue.
159160
/// @param[in] image_decoder The image decoder.
160161
/// @param[in] advisory_script_uri The advisory script uri. This is
161162
/// only used in instrumentation.
@@ -196,6 +197,7 @@ class DartIsolate : public UIDartState {
196197
TaskRunners task_runners,
197198
std::unique_ptr<Window> window,
198199
fml::WeakPtr<IOManager> io_manager,
200+
fml::RefPtr<SkiaUnrefQueue> skia_unref_queue,
199201
fml::WeakPtr<ImageDecoder> image_decoder,
200202
std::string advisory_script_uri,
201203
std::string advisory_script_entrypoint,
@@ -441,6 +443,7 @@ class DartIsolate : public UIDartState {
441443
fml::RefPtr<const DartSnapshot> shared_snapshot,
442444
TaskRunners task_runners,
443445
fml::WeakPtr<IOManager> io_manager,
446+
fml::RefPtr<SkiaUnrefQueue> unref_queue,
444447
fml::WeakPtr<ImageDecoder> image_decoder,
445448
std::string advisory_script_uri,
446449
std::string advisory_script_entrypoint,

‎runtime/dart_isolate_unittests.cc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) {
4242
std::move(task_runners), // task runners
4343
nullptr, // window
4444
{}, // io manager
45+
{}, // unref queue
4546
{}, // image decoder
4647
"main.dart", // advisory uri
4748
"main", // advisory entrypoint,
@@ -75,6 +76,7 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) {
7576
std::move(task_runners), // task runners
7677
nullptr, // window
7778
{}, // io manager
79+
{}, // unref queue
7880
{}, // image decoder
7981
"main.dart", // advisory uri
8082
"main", // advisory entrypoint
@@ -185,6 +187,7 @@ static void RunDartCodeInIsolate(DartVMRef& vm_ref,
185187
std::move(task_runners), // task runners
186188
nullptr, // window
187189
{}, // io manager
190+
{}, // unref queue
188191
{}, // image decoder
189192
"main.dart", // advisory uri
190193
"main", // advisory entrypoint

‎runtime/dart_lifecycle_unittests.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ static std::shared_ptr<DartIsolate> CreateAndRunRootIsolate(
5757
runners, // task_runners
5858
{}, // window
5959
{}, // io_manager
60+
{}, // unref_queue
6061
{}, // image_decoder
6162
"main.dart", // advisory_script_uri
6263
entrypoint.c_str(), // advisory_script_entrypoint

‎runtime/runtime_controller.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ RuntimeController::RuntimeController(
2121
fml::RefPtr<const DartSnapshot> p_shared_snapshot,
2222
TaskRunners p_task_runners,
2323
fml::WeakPtr<IOManager> p_io_manager,
24+
fml::RefPtr<SkiaUnrefQueue> p_unref_queue,
2425
fml::WeakPtr<ImageDecoder> p_image_decoder,
2526
std::string p_advisory_script_uri,
2627
std::string p_advisory_script_entrypoint,
@@ -34,6 +35,7 @@ RuntimeController::RuntimeController(
3435
std::move(p_shared_snapshot),
3536
std::move(p_task_runners),
3637
std::move(p_io_manager),
38+
std::move(p_unref_queue),
3739
std::move(p_image_decoder),
3840
std::move(p_advisory_script_uri),
3941
std::move(p_advisory_script_entrypoint),
@@ -50,6 +52,7 @@ RuntimeController::RuntimeController(
5052
fml::RefPtr<const DartSnapshot> p_shared_snapshot,
5153
TaskRunners p_task_runners,
5254
fml::WeakPtr<IOManager> p_io_manager,
55+
fml::RefPtr<SkiaUnrefQueue> p_unref_queue,
5356
fml::WeakPtr<ImageDecoder> p_image_decoder,
5457
std::string p_advisory_script_uri,
5558
std::string p_advisory_script_entrypoint,
@@ -64,6 +67,7 @@ RuntimeController::RuntimeController(
6467
shared_snapshot_(std::move(p_shared_snapshot)),
6568
task_runners_(p_task_runners),
6669
io_manager_(p_io_manager),
70+
unref_queue_(p_unref_queue),
6771
image_decoder_(p_image_decoder),
6872
advisory_script_uri_(p_advisory_script_uri),
6973
advisory_script_entrypoint_(p_advisory_script_entrypoint),
@@ -82,6 +86,7 @@ RuntimeController::RuntimeController(
8286
task_runners_, //
8387
std::make_unique<Window>(this), //
8488
io_manager_, //
89+
unref_queue_, //
8590
image_decoder_, //
8691
p_advisory_script_uri, //
8792
p_advisory_script_entrypoint, //
@@ -142,6 +147,7 @@ std::unique_ptr<RuntimeController> RuntimeController::Clone() const {
142147
shared_snapshot_, //
143148
task_runners_, //
144149
io_manager_, //
150+
unref_queue_, //
145151
image_decoder_, //
146152
advisory_script_uri_, //
147153
advisory_script_entrypoint_, //

‎runtime/runtime_controller.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ class RuntimeController final : public WindowClient {
3535
fml::RefPtr<const DartSnapshot> shared_snapshot,
3636
TaskRunners task_runners,
3737
fml::WeakPtr<IOManager> io_manager,
38-
fml::WeakPtr<ImageDecoder> iamge_decoder,
38+
fml::RefPtr<SkiaUnrefQueue> unref_queue,
39+
fml::WeakPtr<ImageDecoder> image_decoder,
3940
std::string advisory_script_uri,
4041
std::string advisory_script_entrypoint,
4142
std::function<void(int64_t)> idle_notification_callback,
@@ -131,6 +132,7 @@ class RuntimeController final : public WindowClient {
131132
fml::RefPtr<const DartSnapshot> shared_snapshot_;
132133
TaskRunners task_runners_;
133134
fml::WeakPtr<IOManager> io_manager_;
135+
fml::RefPtr<SkiaUnrefQueue> unref_queue_;
134136
fml::WeakPtr<ImageDecoder> image_decoder_;
135137
std::string advisory_script_uri_;
136138
std::string advisory_script_entrypoint_;
@@ -149,6 +151,7 @@ class RuntimeController final : public WindowClient {
149151
fml::RefPtr<const DartSnapshot> shared_snapshot,
150152
TaskRunners task_runners,
151153
fml::WeakPtr<IOManager> io_manager,
154+
fml::RefPtr<SkiaUnrefQueue> unref_queue,
152155
fml::WeakPtr<ImageDecoder> image_decoder,
153156
std::string advisory_script_uri,
154157
std::string advisory_script_entrypoint,

‎shell/common/engine.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ Engine::Engine(Delegate& delegate,
4343
TaskRunners task_runners,
4444
Settings settings,
4545
std::unique_ptr<Animator> animator,
46-
fml::WeakPtr<IOManager> io_manager)
46+
fml::WeakPtr<IOManager> io_manager,
47+
fml::RefPtr<SkiaUnrefQueue> unref_queue)
4748
: delegate_(delegate),
4849
settings_(std::move(settings)),
4950
animator_(std::move(animator)),
@@ -64,6 +65,7 @@ Engine::Engine(Delegate& delegate,
6465
std::move(shared_snapshot), // shared snapshot
6566
task_runners_, // task runners
6667
std::move(io_manager), // io manager
68+
std::move(unref_queue), // Skia unref queue
6769
image_decoder_.GetWeakPtr(), // image decoder
6870
settings_.advisory_script_uri, // advisory script uri
6971
settings_.advisory_script_entrypoint, // advisory script entrypoint

‎shell/common/engine.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,8 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
280280
TaskRunners task_runners,
281281
Settings settings,
282282
std::unique_ptr<Animator> animator,
283-
fml::WeakPtr<IOManager> io_manager);
283+
fml::WeakPtr<IOManager> io_manager,
284+
fml::RefPtr<SkiaUnrefQueue> unref_queue);
284285

285286
//----------------------------------------------------------------------------
286287
/// @brief Destroys the engine engine. Called by the shell on the UI task

0 commit comments

Comments
 (0)