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

Commit f4b2f32

Browse files
author
Kaushik Iska
committed
[fuchsia] SceneHostBindings are no longer thread locals
Prior to this change SceneHostBindinds was a ThreadLocal but the intention was for it to be IsolateLocal. Given that dart could collect this map on a non-UI thread this caused use-after-free issues. This change fixes it by making it keyed on isolate and koid this is not the ideal solution, this would exist on dart isolate group data struct. Given that Fuchsia is moving to use the embedder API, the decision to use this temporary work around was made. fixes flutter/flutter#49738
1 parent 83a64b7 commit f4b2f32

File tree

1 file changed

+33
-21
lines changed

1 file changed

+33
-21
lines changed

lib/ui/compositing/scene_host.cc

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,33 +10,49 @@
1010
#include <third_party/tonic/dart_binding_macros.h>
1111
#include <third_party/tonic/logging/dart_invoke.h>
1212

13+
#include "dart/runtime/include/dart_api.h"
1314
#include "flutter/flow/view_holder.h"
1415
#include "flutter/fml/thread_local.h"
1516
#include "flutter/lib/ui/ui_dart_state.h"
1617

1718
namespace {
1819

19-
using SceneHostBindings = std::unordered_map<zx_koid_t, flutter::SceneHost*>;
20+
struct SceneHostBindingKey {
21+
Dart_Isolate associated_isolate;
22+
zx_koid_t koid;
2023

21-
FML_THREAD_LOCAL fml::ThreadLocalUniquePtr<SceneHostBindings>
22-
tls_scene_host_bindings;
24+
SceneHostBindingKey(const zx_koid_t koid) {
25+
this->koid = koid;
26+
this->associated_isolate = Dart_CurrentIsolate();
27+
}
2328

24-
void SceneHost_constructor(Dart_NativeArguments args) {
25-
// This UI thread / Isolate contains at least 1 SceneHost. Initialize the
26-
// per-Isolate bindings.
27-
if (tls_scene_host_bindings.get() == nullptr) {
28-
tls_scene_host_bindings.reset(new SceneHostBindings());
29+
bool operator==(const SceneHostBindingKey& other) const {
30+
return associated_isolate == other.associated_isolate && koid == other.koid;
2931
}
32+
};
33+
34+
struct SceneHostBindingKeyHasher {
35+
std::size_t operator()(const SceneHostBindingKey& key) const {
36+
std::size_t koid_hash = std::hash<zx_koid_t>()(key.koid);
37+
std::size_t isolate_hash =
38+
std::hash<Dart_Isolate>()(key.associated_isolate);
39+
return koid_hash ^ isolate_hash;
40+
}
41+
};
42+
43+
using SceneHostBindings = std::unordered_map<SceneHostBindingKey,
44+
flutter::SceneHost*,
45+
SceneHostBindingKeyHasher>;
3046

47+
static SceneHostBindings scene_host_bindings;
48+
49+
void SceneHost_constructor(Dart_NativeArguments args) {
3150
tonic::DartCallConstructor(&flutter::SceneHost::Create, args);
3251
}
3352

3453
flutter::SceneHost* GetSceneHost(scenic::ResourceId id) {
35-
auto* bindings = tls_scene_host_bindings.get();
36-
FML_DCHECK(bindings);
37-
38-
auto binding = bindings->find(id);
39-
if (binding != bindings->end()) {
54+
auto binding = scene_host_bindings.find(id);
55+
if (binding != scene_host_bindings.end()) {
4056
return binding->second;
4157
}
4258

@@ -153,11 +169,9 @@ SceneHost::SceneHost(fml::RefPtr<zircon::dart::Handle> viewHolderToken,
153169
// This callback will be posted as a task when the |scenic::ViewHolder|
154170
// resource is created and given an id by the GPU thread.
155171
auto bind_callback = [scene_host = this](scenic::ResourceId id) {
156-
auto* bindings = tls_scene_host_bindings.get();
157-
FML_DCHECK(bindings);
158-
FML_DCHECK(bindings->find(id) == bindings->end());
159-
160-
bindings->emplace(std::make_pair(id, scene_host));
172+
FML_DCHECK(scene_host_bindings.find(id) == scene_host_bindings.end());
173+
const auto key = SceneHostBindingKey(id);
174+
scene_host_bindings.emplace(std::make_pair(key, scene_host));
161175
};
162176

163177
// Pass the raw handle to the GPU thead; destroying a |zircon::dart::Handle|
@@ -175,9 +189,7 @@ SceneHost::SceneHost(fml::RefPtr<zircon::dart::Handle> viewHolderToken,
175189
}
176190

177191
SceneHost::~SceneHost() {
178-
auto* bindings = tls_scene_host_bindings.get();
179-
FML_DCHECK(bindings);
180-
bindings->erase(koid_);
192+
scene_host_bindings.erase(koid_);
181193

182194
gpu_task_runner_->PostTask(
183195
[id = koid_]() { flutter::ViewHolder::Destroy(id); });

0 commit comments

Comments
 (0)