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

Commit 1ae07ab

Browse files
dcharkesdcharkes
committed
WPH migration
1 parent e1c9673 commit 1ae07ab

File tree

11 files changed

+181
-33
lines changed

11 files changed

+181
-33
lines changed

‎runtime/dart_isolate.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,11 @@ void DartIsolate::AddIsolateShutdownCallback(const fml::closure& closure) {
859859
}
860860

861861
void DartIsolate::OnShutdownCallback() {
862+
tonic::DartState* state = tonic::DartState::Current();
863+
if (state != nullptr) {
864+
state->SetIsShuttingDown();
865+
}
866+
862867
{
863868
tonic::DartApiScope api_scope;
864869
Dart_Handle sticky_error = Dart_GetStickyError();

‎shell/platform/embedder/embedder.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1862,8 +1862,7 @@ FlutterEngineResult FlutterEnginePostDartObject(
18621862
dart_object.value.as_external_typed_data.data = buffer;
18631863
dart_object.value.as_external_typed_data.peer = peer;
18641864
dart_object.value.as_external_typed_data.callback =
1865-
+[](void* unused_isolate_callback_data,
1866-
Dart_WeakPersistentHandle unused_handle, void* peer) {
1865+
+[](void* unused_isolate_callback_data, void* peer) {
18671866
auto typed_peer = reinterpret_cast<ExternalTypedDataPeer*>(peer);
18681867
typed_peer->trampoline(typed_peer->user_data);
18691868
delete typed_peer;

‎shell/platform/fuchsia/dart_runner/dart_runner.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ void IsolateShutdownCallback(void* isolate_group_data, void* isolate_data) {
9191
tonic::DartMicrotaskQueue::GetForCurrentThread()->Destroy();
9292
async_loop_quit(loop);
9393
}
94+
95+
auto state =
96+
static_cast<std::shared_ptr<tonic::DartState>*>(isolate_group_data);
97+
state->get()->SetIsShuttingDown();
9498
}
9599

96100
void IsolateGroupCleanupCallback(void* isolate_group_data) {

‎third_party/tonic/BUILD.gn‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ source_set("tonic") {
2929
"dart_persistent_value.h",
3030
"dart_state.cc",
3131
"dart_state.h",
32+
"dart_weak_persistent_value.cc",
33+
"dart_weak_persistent_value.h",
3234
"dart_wrappable.cc",
3335
"dart_wrappable.h",
3436
"dart_wrapper_info.h",

‎third_party/tonic/dart_state.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ DartState::DartState(int dirfd,
2727
message_handler_(new DartMessageHandler()),
2828
file_loader_(new FileLoader(dirfd)),
2929
message_epilogue_(message_epilogue),
30-
has_set_return_code_(false) {}
30+
has_set_return_code_(false),
31+
is_shutting_down_(false) {}
3132

3233
DartState::~DartState() {}
3334

‎third_party/tonic/dart_state.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ class DartState : public std::enable_shared_from_this<DartState> {
6868
void SetReturnCodeCallback(std::function<void(uint32_t)> callback);
6969
bool has_set_return_code() const { return has_set_return_code_; }
7070

71+
void SetIsShuttingDown() { is_shutting_down_ = true; }
72+
bool IsShuttingDown() { return is_shutting_down_; }
73+
7174
virtual void DidSetIsolate();
7275

7376
static Dart_Handle HandleLibraryTag(Dart_LibraryTag tag,
@@ -83,6 +86,7 @@ class DartState : public std::enable_shared_from_this<DartState> {
8386
std::function<void(Dart_Handle)> message_epilogue_;
8487
std::function<void(uint32_t)> set_return_code_callback_;
8588
bool has_set_return_code_;
89+
std::atomic<bool> is_shutting_down_;
8690

8791
protected:
8892
TONIC_DISALLOW_COPY_AND_ASSIGN(DartState);

‎third_party/tonic/dart_weak_persistent_value.cc‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "tonic/dart_weak_persistent_value.h"
6+
7+
#include "tonic/dart_state.h"
8+
#include "tonic/scopes/dart_isolate_scope.h"
9+
10+
namespace tonic {
11+
12+
DartWeakPersistentValue::DartWeakPersistentValue() : handle_(nullptr) {}
13+
14+
DartWeakPersistentValue::~DartWeakPersistentValue() {
15+
Clear();
16+
}
17+
18+
void DartWeakPersistentValue::Set(DartState* dart_state,
19+
Dart_Handle object,
20+
void* peer,
21+
intptr_t external_allocation_size,
22+
Dart_HandleFinalizer callback) {
23+
TONIC_DCHECK(is_empty());
24+
dart_state_ = dart_state->GetWeakPtr();
25+
handle_ = Dart_NewWeakPersistentHandle(object, peer, external_allocation_size,
26+
callback);
27+
}
28+
29+
void DartWeakPersistentValue::Clear() {
30+
if (!handle_) {
31+
return;
32+
}
33+
34+
auto dart_state = dart_state_.lock();
35+
if (!dart_state) {
36+
return;
37+
}
38+
39+
if (!dart_state->IsShuttingDown()) {
40+
if (Dart_CurrentIsolateGroup()) {
41+
Dart_DeleteWeakPersistentHandle(handle_);
42+
} else {
43+
DartIsolateScope scope(dart_state->isolate());
44+
Dart_DeleteWeakPersistentHandle(handle_);
45+
}
46+
}
47+
// If it's shutting down, the handle will be deleted already.
48+
49+
dart_state_.reset();
50+
handle_ = nullptr;
51+
}
52+
53+
Dart_Handle DartWeakPersistentValue::Get() {
54+
auto dart_state = dart_state_.lock();
55+
TONIC_DCHECK(dart_state);
56+
TONIC_DCHECK(!dart_state->IsShuttingDown());
57+
if (!handle_) {
58+
return nullptr;
59+
}
60+
return Dart_HandleFromWeakPersistent(handle_);
61+
}
62+
63+
} // namespace tonic

‎third_party/tonic/dart_weak_persistent_value.h‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#ifndef LIB_TONIC_DART_WEAK_PERSISTENT_VALUE_H_
6+
#define LIB_TONIC_DART_WEAK_PERSISTENT_VALUE_H_
7+
8+
#include <memory>
9+
10+
#include "third_party/dart/runtime/include/dart_api.h"
11+
#include "tonic/common/macros.h"
12+
13+
namespace tonic {
14+
class DartState;
15+
16+
// DartWeakPersistentValue is a bookkeeping class to help pair calls to
17+
// Dart_NewWeakPersistentHandle with Dart_DeleteWeakPersistentHandle even in
18+
// the case if IsolateGroup shutdown. Consider using this class instead of
19+
// holding a Dart_PersistentHandle directly so that you don't leak the
20+
// Dart_WeakPersistentHandle.
21+
class DartWeakPersistentValue {
22+
public:
23+
DartWeakPersistentValue();
24+
~DartWeakPersistentValue();
25+
26+
Dart_WeakPersistentHandle value() const { return handle_; }
27+
bool is_empty() const { return handle_ == nullptr; }
28+
29+
void Set(DartState* dart_state,
30+
Dart_Handle object,
31+
void* peer,
32+
intptr_t external_allocation_size,
33+
Dart_HandleFinalizer callback);
34+
void Clear();
35+
Dart_Handle Get();
36+
37+
const std::weak_ptr<DartState>& dart_state() const { return dart_state_; }
38+
39+
private:
40+
std::weak_ptr<DartState> dart_state_;
41+
Dart_WeakPersistentHandle handle_;
42+
43+
TONIC_DISALLOW_COPY_AND_ASSIGN(DartWeakPersistentValue);
44+
};
45+
46+
} // namespace tonic
47+
48+
#endif // LIB_TONIC_DART_WEAK_PERSISTENT_VALUE_H_

‎third_party/tonic/dart_wrappable.cc‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,17 @@
1212
namespace tonic {
1313

1414
DartWrappable::~DartWrappable() {
15-
TONIC_CHECK(!dart_wrapper_);
15+
// Calls the destructor of dart_wrapper_ to delete WeakPersistentHandle.
1616
}
1717

1818
// TODO(dnfield): Delete this. https://github.com/flutter/flutter/issues/50997
1919
Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) {
20-
TONIC_DCHECK(!dart_wrapper_);
20+
if (!dart_wrapper_.is_empty()) {
21+
// Any previously given out wrapper must have been GCed.
22+
TONIC_DCHECK(Dart_IsNull(dart_wrapper_.Get()));
23+
dart_wrapper_.Clear();
24+
}
25+
2126
const DartWrapperInfo& info = GetDartWrapperInfo();
2227

2328
Dart_PersistentHandle type = dart_state->class_library().GetClass(info);
@@ -36,14 +41,19 @@ Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) {
3641
TONIC_DCHECK(!LogIfError(res));
3742

3843
this->RetainDartWrappableReference(); // Balanced in FinalizeDartWrapper.
39-
dart_wrapper_ = Dart_NewWeakPersistentHandle(
40-
wrapper, this, GetAllocationSize(), &FinalizeDartWrapper);
44+
dart_wrapper_.Set(dart_state, wrapper, this, GetAllocationSize(),
45+
&FinalizeDartWrapper);
4146

4247
return wrapper;
4348
}
4449

4550
void DartWrappable::AssociateWithDartWrapper(Dart_Handle wrapper) {
46-
TONIC_DCHECK(!dart_wrapper_);
51+
if (!dart_wrapper_.is_empty()) {
52+
// Any previously given out wrapper must have been GCed.
53+
TONIC_DCHECK(Dart_IsNull(dart_wrapper_.Get()));
54+
dart_wrapper_.Clear();
55+
}
56+
4757
TONIC_CHECK(!LogIfError(wrapper));
4858

4959
const DartWrapperInfo& info = GetDartWrapperInfo();
@@ -54,26 +64,25 @@ void DartWrappable::AssociateWithDartWrapper(Dart_Handle wrapper) {
5464
wrapper, kWrapperInfoIndex, reinterpret_cast<intptr_t>(&info))));
5565

5666
this->RetainDartWrappableReference(); // Balanced in FinalizeDartWrapper.
57-
dart_wrapper_ = Dart_NewWeakPersistentHandle(
58-
wrapper, this, GetAllocationSize(), &FinalizeDartWrapper);
67+
68+
DartState* dart_state = DartState::Current();
69+
dart_wrapper_.Set(dart_state, wrapper, this, GetAllocationSize(),
70+
&FinalizeDartWrapper);
5971
}
6072

6173
void DartWrappable::ClearDartWrapper() {
62-
TONIC_DCHECK(dart_wrapper_);
63-
Dart_Handle wrapper = Dart_HandleFromWeakPersistent(dart_wrapper_);
74+
TONIC_DCHECK(!dart_wrapper_.is_empty());
75+
Dart_Handle wrapper = dart_wrapper_.Get();
6476
TONIC_CHECK(!LogIfError(Dart_SetNativeInstanceField(wrapper, kPeerIndex, 0)));
6577
TONIC_CHECK(
6678
!LogIfError(Dart_SetNativeInstanceField(wrapper, kWrapperInfoIndex, 0)));
67-
Dart_DeleteWeakPersistentHandle(dart_wrapper_);
68-
dart_wrapper_ = nullptr;
79+
dart_wrapper_.Clear();
6980
this->ReleaseDartWrappableReference();
7081
}
7182

7283
void DartWrappable::FinalizeDartWrapper(void* isolate_callback_data,
73-
Dart_WeakPersistentHandle wrapper,
7484
void* peer) {
7585
DartWrappable* wrappable = reinterpret_cast<DartWrappable*>(peer);
76-
wrappable->dart_wrapper_ = nullptr;
7786
wrappable->ReleaseDartWrappableReference(); // Balanced in CreateDartWrapper.
7887
}
7988

‎third_party/tonic/dart_wrappable.h‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "tonic/common/macros.h"
1010
#include "tonic/converter/dart_converter.h"
1111
#include "tonic/dart_state.h"
12+
#include "tonic/dart_weak_persistent_value.h"
1213
#include "tonic/dart_wrapper_info.h"
1314
#include "tonic/logging/dart_error.h"
1415

@@ -26,7 +27,7 @@ class DartWrappable {
2627
kNumberOfNativeFields,
2728
};
2829

29-
DartWrappable() : dart_wrapper_(nullptr) {}
30+
DartWrappable() : dart_wrapper_(DartWeakPersistentValue()) {}
3031

3132
// Subclasses that wish to expose a new interface must override this function
3233
// and provide information about their wrapper. There is no need to call your
@@ -49,7 +50,9 @@ class DartWrappable {
4950
Dart_Handle CreateDartWrapper(DartState* dart_state);
5051
void AssociateWithDartWrapper(Dart_Handle wrappable);
5152
void ClearDartWrapper(); // Warning: Might delete this.
52-
Dart_WeakPersistentHandle dart_wrapper() const { return dart_wrapper_; }
53+
Dart_WeakPersistentHandle dart_wrapper() const {
54+
return dart_wrapper_.value();
55+
}
5356

5457
protected:
5558
virtual ~DartWrappable();
@@ -59,11 +62,9 @@ class DartWrappable {
5962
const tonic::DartWrapperInfo& wrapper_info);
6063

6164
private:
62-
static void FinalizeDartWrapper(void* isolate_callback_data,
63-
Dart_WeakPersistentHandle wrapper,
64-
void* peer);
65+
static void FinalizeDartWrapper(void* isolate_callback_data, void* peer);
6566

66-
Dart_WeakPersistentHandle dart_wrapper_;
67+
DartWeakPersistentValue dart_wrapper_;
6768

6869
TONIC_DISALLOW_COPY_AND_ASSIGN(DartWrappable);
6970
};
@@ -103,22 +104,36 @@ struct DartConverter<
103104
typename std::enable_if<
104105
std::is_convertible<T*, const DartWrappable*>::value>::type> {
105106
static Dart_Handle ToDart(DartWrappable* val) {
106-
if (!val)
107+
if (!val) {
107108
return Dart_Null();
108-
if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper())
109-
return Dart_HandleFromWeakPersistent(wrapper);
109+
}
110+
if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper()) {
111+
auto strong_handle = Dart_HandleFromWeakPersistent(wrapper);
112+
if (!Dart_IsNull(strong_handle)) {
113+
return strong_handle;
114+
}
115+
// After the weak referenced object has been GCed, the handle points to
116+
// Dart_Null(). Fall through create a new wrapper object.
117+
}
110118
return val->CreateDartWrapper(DartState::Current());
111119
}
112120

113121
static void SetReturnValue(Dart_NativeArguments args,
114122
DartWrappable* val,
115123
bool auto_scope = true) {
116-
if (!val)
124+
if (!val) {
117125
Dart_SetReturnValue(args, Dart_Null());
118-
else if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper())
119-
Dart_SetWeakHandleReturnValue(args, wrapper);
120-
else
121-
Dart_SetReturnValue(args, val->CreateDartWrapper(DartState::Current()));
126+
return;
127+
} else if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper()) {
128+
auto strong_handle = Dart_HandleFromWeakPersistent(wrapper);
129+
if (!Dart_IsNull(strong_handle)) {
130+
Dart_SetReturnValue(args, strong_handle);
131+
return;
132+
}
133+
// After the weak referenced object has been GCed, the handle points to
134+
// Dart_Null(). Fall through create a new wrapper object.
135+
}
136+
Dart_SetReturnValue(args, val->CreateDartWrapper(DartState::Current()));
122137
}
123138

124139
static T* FromDart(Dart_Handle handle) {

0 commit comments

Comments
 (0)