diff --git a/BUILD.gn b/BUILD.gn index edbeb6803c3e51587736e72ab0283a69d20260ff..e280ae9019179d06ccfbfa66218953d599e05616 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -98,6 +98,7 @@ group("flutter") { "//flutter/shell/platform/embedder:embedder_proctable_unittests", "//flutter/shell/platform/embedder:embedder_unittests", "//flutter/testing:testing_unittests", + "//flutter/third_party/tonic/tests:tonic_unittests", "//flutter/third_party/txt:txt_unittests", ] diff --git a/DEPS b/DEPS index 1cce4cac1d3073462485865125a217e3ee8bd2b6..7294490d95b7c9a373305b646573b9a9f9449472 100644 --- a/DEPS +++ b/DEPS @@ -34,7 +34,7 @@ vars = { # Dart is: https://github.com/dart-lang/sdk/blob/master/DEPS. # You can use //tools/dart/create_updated_flutter_deps.py to produce # updated revision list of existing dependencies. - 'dart_revision': 'd2577410a5016eadc111919355e19d42dd45d816', + 'dart_revision': '52783837369de45d3372cb6c6b7cdd63e71cd829', # WARNING: DO NOT EDIT MANUALLY # The lines between blank lines above and below are generated by a script. See create_updated_flutter_deps.py diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index d4b2c65b71969e519ad024f97fe53fce80f3afd6..f00657c21589b4dc4e57a69d6e6bb28412cd1a28 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1463,6 +1463,8 @@ FILE: ../../../flutter/third_party/tonic/dart_persistent_value.cc FILE: ../../../flutter/third_party/tonic/dart_persistent_value.h FILE: ../../../flutter/third_party/tonic/dart_state.cc FILE: ../../../flutter/third_party/tonic/dart_state.h +FILE: ../../../flutter/third_party/tonic/dart_weak_persistent_value.cc +FILE: ../../../flutter/third_party/tonic/dart_weak_persistent_value.h FILE: ../../../flutter/third_party/tonic/dart_wrappable.cc FILE: ../../../flutter/third_party/tonic/dart_wrappable.h FILE: ../../../flutter/third_party/tonic/dart_wrapper_info.h diff --git a/ci/licenses_golden/licenses_third_party b/ci/licenses_golden/licenses_third_party index 3e60c512c0e079292dd53724b4934feb6ea600b8..371a84961b2d6f01bab7597b9f65d015a7b157d3 100644 --- a/ci/licenses_golden/licenses_third_party +++ b/ci/licenses_golden/licenses_third_party @@ -1,4 +1,4 @@ -Signature: c55e82fda8f939e4cdbeb9c2c004e7d9 +Signature: 51a79de001f82d05f084b671f6216a31 UNUSED LICENSES: diff --git a/lib/ui/painting/image_decoder.cc b/lib/ui/painting/image_decoder.cc index d6c4a668fb5fc18f72748072f51f2a926c4b9639..b6ffb20f5ee7e14452b8ed5028cca322cab95c79 100644 --- a/lib/ui/painting/image_decoder.cc +++ b/lib/ui/painting/image_decoder.cc @@ -224,12 +224,13 @@ void ImageDecoder::Decode(fml::RefPtr descriptor, FML_DCHECK(callback); FML_DCHECK(runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); - // Always service the callback on the UI thread. - auto result = [callback, ui_runner = runners_.GetUITaskRunner()]( + // Always service the callback (and cleanup the descriptor) on the UI thread. + auto result = [callback, descriptor, ui_runner = runners_.GetUITaskRunner()]( SkiaGPUObject image, fml::tracing::TraceFlow flow) { - ui_runner->PostTask(fml::MakeCopyable( - [callback, image = std::move(image), flow = std::move(flow)]() mutable { + ui_runner->PostTask( + fml::MakeCopyable([callback, descriptor, image = std::move(image), + flow = std::move(flow)]() mutable { // We are going to terminate the trace flow here. Flows cannot // terminate without a base trace. Add one explicitly. TRACE_EVENT0("flutter", "ImageDecodeCallback"); diff --git a/lib/ui/painting/image_encoding.cc b/lib/ui/painting/image_encoding.cc index c17a784c54caf7faea0e697293914778b8198af2..8a7653f0b0bbbf7f59bdddf82c05653eed8c0397 100644 --- a/lib/ui/painting/image_encoding.cc +++ b/lib/ui/painting/image_encoding.cc @@ -35,9 +35,7 @@ enum ImageByteFormat { kPNG, }; -void FinalizeSkData(void* isolate_callback_data, - Dart_WeakPersistentHandle handle, - void* peer) { +void FinalizeSkData(void* isolate_callback_data, void* peer) { SkData* buffer = reinterpret_cast(peer); buffer->unref(); } diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index b834957a9281af3b9651c3eb8642cc47b35f2e20..8a6215110e42600727e5e75ae877ab3253aabdfd 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -981,6 +981,11 @@ void DartIsolate::AddIsolateShutdownCallback(const fml::closure& closure) { } void DartIsolate::OnShutdownCallback() { + tonic::DartState* state = tonic::DartState::Current(); + if (state != nullptr) { + state->SetIsShuttingDown(); + } + { tonic::DartApiScope api_scope; Dart_Handle sticky_error = Dart_GetStickyError(); diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc index ae9849a5a328a2b27c543f9e501e0d8351425902..b0c61a1b58c79917cb7e9529723430352a8634bc 100644 --- a/shell/platform/embedder/embedder.cc +++ b/shell/platform/embedder/embedder.cc @@ -1922,8 +1922,7 @@ FlutterEngineResult FlutterEnginePostDartObject( dart_object.value.as_external_typed_data.data = buffer; dart_object.value.as_external_typed_data.peer = peer; dart_object.value.as_external_typed_data.callback = - +[](void* unused_isolate_callback_data, - Dart_WeakPersistentHandle unused_handle, void* peer) { + +[](void* unused_isolate_callback_data, void* peer) { auto typed_peer = reinterpret_cast(peer); typed_peer->trampoline(typed_peer->user_data); delete typed_peer; diff --git a/shell/platform/fuchsia/dart_runner/dart_runner.cc b/shell/platform/fuchsia/dart_runner/dart_runner.cc index 76d85405ab47410f19bb0be2cba4b25359ed85c9..415d2885c60c2ec97e4b275e33adef727ecf10a8 100644 --- a/shell/platform/fuchsia/dart_runner/dart_runner.cc +++ b/shell/platform/fuchsia/dart_runner/dart_runner.cc @@ -86,6 +86,10 @@ void IsolateShutdownCallback(void* isolate_group_data, void* isolate_data) { tonic::DartMicrotaskQueue::GetForCurrentThread()->Destroy(); async_loop_quit(loop); } + + auto state = + static_cast*>(isolate_group_data); + state->get()->SetIsShuttingDown(); } void IsolateGroupCleanupCallback(void* isolate_group_data) { diff --git a/testing/run_tests.py b/testing/run_tests.py index 06b8256d0bc8a4624d34fcd74a8b0a0527621ae3..8bdd114f43988b648902e1622f698b2f0a039c29 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -132,6 +132,8 @@ def RunCCTests(build_dir, filter): RunEngineExecutable(build_dir, 'runtime_unittests', filter, shuffle_flags) + RunEngineExecutable(build_dir, 'tonic_unittests', filter, shuffle_flags) + if not IsWindows(): # https://github.com/flutter/flutter/issues/36295 RunEngineExecutable(build_dir, 'shell_unittests', filter, shuffle_flags) diff --git a/third_party/tonic/BUILD.gn b/third_party/tonic/BUILD.gn index f5bedda8dab0d3a484cf7698762692d6fe07d104..9a6abda8ba6959af3024134b451afc56a03a0baf 100644 --- a/third_party/tonic/BUILD.gn +++ b/third_party/tonic/BUILD.gn @@ -35,6 +35,8 @@ source_set("tonic") { "dart_persistent_value.h", "dart_state.cc", "dart_state.h", + "dart_weak_persistent_value.cc", + "dart_weak_persistent_value.h", "dart_wrappable.cc", "dart_wrappable.h", "dart_wrapper_info.h", diff --git a/third_party/tonic/dart_state.cc b/third_party/tonic/dart_state.cc index b711a229777883e2136af04e099d0ef09dbb6ffd..3f37685c51be2c547033e97864102841856023a3 100644 --- a/third_party/tonic/dart_state.cc +++ b/third_party/tonic/dart_state.cc @@ -27,7 +27,8 @@ DartState::DartState(int dirfd, message_handler_(new DartMessageHandler()), file_loader_(new FileLoader(dirfd)), message_epilogue_(message_epilogue), - has_set_return_code_(false) {} + has_set_return_code_(false), + is_shutting_down_(false) {} DartState::~DartState() {} diff --git a/third_party/tonic/dart_state.h b/third_party/tonic/dart_state.h index 845914937dcd01a5c4f1a68d0a357fd5b67ce792..1984a66bfb88889ecd933cbb18ed07931f02a55e 100644 --- a/third_party/tonic/dart_state.h +++ b/third_party/tonic/dart_state.h @@ -5,6 +5,7 @@ #ifndef LIB_TONIC_DART_STATE_H_ #define LIB_TONIC_DART_STATE_H_ +#include #include #include @@ -68,6 +69,9 @@ class DartState : public std::enable_shared_from_this { void SetReturnCodeCallback(std::function callback); bool has_set_return_code() const { return has_set_return_code_; } + void SetIsShuttingDown() { is_shutting_down_ = true; } + bool IsShuttingDown() { return is_shutting_down_; } + virtual void DidSetIsolate(); static Dart_Handle HandleLibraryTag(Dart_LibraryTag tag, @@ -83,6 +87,7 @@ class DartState : public std::enable_shared_from_this { std::function message_epilogue_; std::function set_return_code_callback_; bool has_set_return_code_; + std::atomic is_shutting_down_; protected: TONIC_DISALLOW_COPY_AND_ASSIGN(DartState); diff --git a/third_party/tonic/dart_weak_persistent_value.cc b/third_party/tonic/dart_weak_persistent_value.cc new file mode 100644 index 0000000000000000000000000000000000000000..a2664d3e07990e3c1828dff687e768b13ca21195 --- /dev/null +++ b/third_party/tonic/dart_weak_persistent_value.cc @@ -0,0 +1,70 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "tonic/dart_weak_persistent_value.h" + +#include "tonic/dart_state.h" +#include "tonic/scopes/dart_isolate_scope.h" + +namespace tonic { + +DartWeakPersistentValue::DartWeakPersistentValue() : handle_(nullptr) {} + +DartWeakPersistentValue::~DartWeakPersistentValue() { + Clear(); +} + +void DartWeakPersistentValue::Set(DartState* dart_state, + Dart_Handle object, + void* peer, + intptr_t external_allocation_size, + Dart_HandleFinalizer callback) { + TONIC_DCHECK(is_empty()); + dart_state_ = dart_state->GetWeakPtr(); + handle_ = Dart_NewWeakPersistentHandle(object, peer, external_allocation_size, + callback); +} + +void DartWeakPersistentValue::Clear() { + if (!handle_) { + return; + } + + auto dart_state = dart_state_.lock(); + if (!dart_state) { + // The DartVM that the handle used to belong to has been shut down and that + // handle has already been deleted. + handle_ = nullptr; + return; + } + + // The DartVM frees the handles during shutdown and calls the finalizers. + // Freeing handles during shutdown would fail. + if (!dart_state->IsShuttingDown()) { + if (Dart_CurrentIsolateGroup()) { + Dart_DeleteWeakPersistentHandle(handle_); + } else { + // If we are not on the mutator thread, this will fail. The caller must + // ensure to be on the mutator thread. + DartIsolateScope scope(dart_state->isolate()); + Dart_DeleteWeakPersistentHandle(handle_); + } + } + // If it's shutting down, the handle will be deleted already. + + dart_state_.reset(); + handle_ = nullptr; +} + +Dart_Handle DartWeakPersistentValue::Get() { + auto dart_state = dart_state_.lock(); + TONIC_DCHECK(dart_state); + TONIC_DCHECK(!dart_state->IsShuttingDown()); + if (!handle_) { + return nullptr; + } + return Dart_HandleFromWeakPersistent(handle_); +} + +} // namespace tonic diff --git a/third_party/tonic/dart_weak_persistent_value.h b/third_party/tonic/dart_weak_persistent_value.h new file mode 100644 index 0000000000000000000000000000000000000000..5f8aed5ee6ea2ddc2e86d5b1b9c7822f3cadd343 --- /dev/null +++ b/third_party/tonic/dart_weak_persistent_value.h @@ -0,0 +1,48 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef LIB_TONIC_DART_WEAK_PERSISTENT_VALUE_H_ +#define LIB_TONIC_DART_WEAK_PERSISTENT_VALUE_H_ + +#include + +#include "third_party/dart/runtime/include/dart_api.h" +#include "tonic/common/macros.h" + +namespace tonic { +class DartState; + +// DartWeakPersistentValue is a bookkeeping class to help pair calls to +// Dart_NewWeakPersistentHandle with Dart_DeleteWeakPersistentHandle even in +// the case if IsolateGroup shutdown. Consider using this class instead of +// holding a Dart_PersistentHandle directly so that you don't leak the +// Dart_WeakPersistentHandle. +class DartWeakPersistentValue { + public: + DartWeakPersistentValue(); + ~DartWeakPersistentValue(); + + Dart_WeakPersistentHandle value() const { return handle_; } + bool is_empty() const { return handle_ == nullptr; } + + void Set(DartState* dart_state, + Dart_Handle object, + void* peer, + intptr_t external_allocation_size, + Dart_HandleFinalizer callback); + void Clear(); + Dart_Handle Get(); + + const std::weak_ptr& dart_state() const { return dart_state_; } + + private: + std::weak_ptr dart_state_; + Dart_WeakPersistentHandle handle_; + + TONIC_DISALLOW_COPY_AND_ASSIGN(DartWeakPersistentValue); +}; + +} // namespace tonic + +#endif // LIB_TONIC_DART_WEAK_PERSISTENT_VALUE_H_ diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index 3bdfe3e6e498a66b5e689e56bff8196ae6453e8f..858215c1196a9c06faa6eea8a2076ce30e8f23ce 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -12,12 +12,17 @@ namespace tonic { DartWrappable::~DartWrappable() { - TONIC_CHECK(!dart_wrapper_); + // Calls the destructor of dart_wrapper_ to delete WeakPersistentHandle. } // TODO(dnfield): Delete this. https://github.com/flutter/flutter/issues/50997 Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { - TONIC_DCHECK(!dart_wrapper_); + if (!dart_wrapper_.is_empty()) { + // Any previously given out wrapper must have been GCed. + TONIC_DCHECK(Dart_IsNull(dart_wrapper_.Get())); + dart_wrapper_.Clear(); + } + const DartWrapperInfo& info = GetDartWrapperInfo(); Dart_PersistentHandle type = dart_state->class_library().GetClass(info); @@ -36,14 +41,19 @@ Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { TONIC_DCHECK(!LogIfError(res)); this->RetainDartWrappableReference(); // Balanced in FinalizeDartWrapper. - dart_wrapper_ = Dart_NewWeakPersistentHandle( - wrapper, this, GetAllocationSize(), &FinalizeDartWrapper); + dart_wrapper_.Set(dart_state, wrapper, this, GetAllocationSize(), + &FinalizeDartWrapper); return wrapper; } void DartWrappable::AssociateWithDartWrapper(Dart_Handle wrapper) { - TONIC_DCHECK(!dart_wrapper_); + if (!dart_wrapper_.is_empty()) { + // Any previously given out wrapper must have been GCed. + TONIC_DCHECK(Dart_IsNull(dart_wrapper_.Get())); + dart_wrapper_.Clear(); + } + TONIC_CHECK(!LogIfError(wrapper)); const DartWrapperInfo& info = GetDartWrapperInfo(); @@ -54,26 +64,25 @@ void DartWrappable::AssociateWithDartWrapper(Dart_Handle wrapper) { wrapper, kWrapperInfoIndex, reinterpret_cast(&info)))); this->RetainDartWrappableReference(); // Balanced in FinalizeDartWrapper. - dart_wrapper_ = Dart_NewWeakPersistentHandle( - wrapper, this, GetAllocationSize(), &FinalizeDartWrapper); + + DartState* dart_state = DartState::Current(); + dart_wrapper_.Set(dart_state, wrapper, this, GetAllocationSize(), + &FinalizeDartWrapper); } void DartWrappable::ClearDartWrapper() { - TONIC_DCHECK(dart_wrapper_); - Dart_Handle wrapper = Dart_HandleFromWeakPersistent(dart_wrapper_); + TONIC_DCHECK(!dart_wrapper_.is_empty()); + Dart_Handle wrapper = dart_wrapper_.Get(); TONIC_CHECK(!LogIfError(Dart_SetNativeInstanceField(wrapper, kPeerIndex, 0))); TONIC_CHECK( !LogIfError(Dart_SetNativeInstanceField(wrapper, kWrapperInfoIndex, 0))); - Dart_DeleteWeakPersistentHandle(dart_wrapper_); - dart_wrapper_ = nullptr; + dart_wrapper_.Clear(); this->ReleaseDartWrappableReference(); } void DartWrappable::FinalizeDartWrapper(void* isolate_callback_data, - Dart_WeakPersistentHandle wrapper, void* peer) { DartWrappable* wrappable = reinterpret_cast(peer); - wrappable->dart_wrapper_ = nullptr; wrappable->ReleaseDartWrappableReference(); // Balanced in CreateDartWrapper. } diff --git a/third_party/tonic/dart_wrappable.h b/third_party/tonic/dart_wrappable.h index a036abb854e7c21c1c8a9bec9911356dac7d50f2..f944dacefbfac789f2d494c27e677a6e413823a0 100644 --- a/third_party/tonic/dart_wrappable.h +++ b/third_party/tonic/dart_wrappable.h @@ -9,6 +9,7 @@ #include "tonic/common/macros.h" #include "tonic/converter/dart_converter.h" #include "tonic/dart_state.h" +#include "tonic/dart_weak_persistent_value.h" #include "tonic/dart_wrapper_info.h" #include "tonic/logging/dart_error.h" @@ -26,7 +27,7 @@ class DartWrappable { kNumberOfNativeFields, }; - DartWrappable() : dart_wrapper_(nullptr) {} + DartWrappable() : dart_wrapper_(DartWeakPersistentValue()) {} // Subclasses that wish to expose a new interface must override this function // and provide information about their wrapper. There is no need to call your @@ -49,7 +50,9 @@ class DartWrappable { Dart_Handle CreateDartWrapper(DartState* dart_state); void AssociateWithDartWrapper(Dart_Handle wrappable); void ClearDartWrapper(); // Warning: Might delete this. - Dart_WeakPersistentHandle dart_wrapper() const { return dart_wrapper_; } + Dart_WeakPersistentHandle dart_wrapper() const { + return dart_wrapper_.value(); + } protected: virtual ~DartWrappable(); @@ -59,11 +62,9 @@ class DartWrappable { const tonic::DartWrapperInfo& wrapper_info); private: - static void FinalizeDartWrapper(void* isolate_callback_data, - Dart_WeakPersistentHandle wrapper, - void* peer); + static void FinalizeDartWrapper(void* isolate_callback_data, void* peer); - Dart_WeakPersistentHandle dart_wrapper_; + DartWeakPersistentValue dart_wrapper_; TONIC_DISALLOW_COPY_AND_ASSIGN(DartWrappable); }; @@ -103,22 +104,36 @@ struct DartConverter< typename std::enable_if< std::is_convertible::value>::type> { static Dart_Handle ToDart(DartWrappable* val) { - if (!val) + if (!val) { return Dart_Null(); - if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper()) - return Dart_HandleFromWeakPersistent(wrapper); + } + if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper()) { + auto strong_handle = Dart_HandleFromWeakPersistent(wrapper); + if (!Dart_IsNull(strong_handle)) { + return strong_handle; + } + // After the weak referenced object has been GCed, the handle points to + // Dart_Null(). Fall through create a new wrapper object. + } return val->CreateDartWrapper(DartState::Current()); } static void SetReturnValue(Dart_NativeArguments args, DartWrappable* val, bool auto_scope = true) { - if (!val) + if (!val) { Dart_SetReturnValue(args, Dart_Null()); - else if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper()) - Dart_SetWeakHandleReturnValue(args, wrapper); - else - Dart_SetReturnValue(args, val->CreateDartWrapper(DartState::Current())); + return; + } else if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper()) { + auto strong_handle = Dart_HandleFromWeakPersistent(wrapper); + if (!Dart_IsNull(strong_handle)) { + Dart_SetReturnValue(args, strong_handle); + return; + } + // After the weak referenced object has been GCed, the handle points to + // Dart_Null(). Fall through create a new wrapper object. + } + Dart_SetReturnValue(args, val->CreateDartWrapper(DartState::Current())); } static T* FromDart(Dart_Handle handle) { diff --git a/third_party/tonic/tests/BUILD.gn b/third_party/tonic/tests/BUILD.gn new file mode 100644 index 0000000000000000000000000000000000000000..359deb13b7904e56906f0ab52008da153e6c7a92 --- /dev/null +++ b/third_party/tonic/tests/BUILD.gn @@ -0,0 +1,33 @@ +# Copyright 2013 The Flutter Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import("//flutter/testing/testing.gni") + +test_fixtures("tonic_fixtures") { + dart_main = "fixtures/tonic_test.dart" + fixtures = [] +} + +executable("tonic_unittests") { + testonly = true + + public_configs = [ "//flutter:export_dynamic_symbols" ] + + sources = [ + "dart_state_unittest.cc", + "dart_weak_persistent_handle_unittest.cc", + ] + + public_deps = [ + ":tonic_fixtures", + "//flutter/runtime:libdart", + "//flutter/runtime:runtime", + "//flutter/testing", + "//flutter/testing:fixture_test", + "//third_party/dart/runtime:dart_api", + "//third_party/googletest:gtest", + ] + + deps = [ "../:tonic" ] +} diff --git a/third_party/tonic/tests/dart_state_unittest.cc b/third_party/tonic/tests/dart_state_unittest.cc new file mode 100644 index 0000000000000000000000000000000000000000..3d053de40e75b17d6c29cad52107142635253533 --- /dev/null +++ b/third_party/tonic/tests/dart_state_unittest.cc @@ -0,0 +1,61 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/common/task_runners.h" +#include "flutter/runtime/dart_vm_lifecycle.h" +#include "flutter/runtime/isolate_configuration.h" +#include "flutter/testing/fixture_test.h" + +namespace flutter { +namespace testing { + +using DartState = FixtureTest; + +TEST_F(DartState, IsShuttingDown) { + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + auto settings = CreateSettingsForFixture(); + auto vm_ref = DartVMRef::Create(settings); + ASSERT_TRUE(vm_ref); + auto vm_data = vm_ref.GetVMData(); + ASSERT_TRUE(vm_data); + TaskRunners task_runners(GetCurrentTestName(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner() // + ); + auto isolate_configuration = + IsolateConfiguration::InferFromSettings(settings); + auto weak_isolate = DartIsolate::CreateRunningRootIsolate( + vm_data->GetSettings(), // settings + vm_data->GetIsolateSnapshot(), // isolate snapshot + std::move(task_runners), // task runners + nullptr, // window + {}, // snapshot delegate + {}, // hint freed delegate + {}, // io manager + {}, // unref queue + {}, // image decoder + "main.dart", // advisory uri + "main", // advisory entrypoint + DartIsolate::Flags{}, // flags + settings.isolate_create_callback, // isolate create callback + settings.isolate_shutdown_callback, // isolate shutdown callback + "main", // dart entrypoint + std::nullopt, // dart entrypoint library + std::move(isolate_configuration) // isolate configuration + ); + auto root_isolate = weak_isolate.lock(); + ASSERT_TRUE(root_isolate); + + tonic::DartState* dart_state = root_isolate.get(); + ASSERT_FALSE(dart_state->IsShuttingDown()); + + ASSERT_TRUE(root_isolate->Shutdown()); + + ASSERT_TRUE(dart_state->IsShuttingDown()); +} + +} // namespace testing +} // namespace flutter diff --git a/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc b/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc new file mode 100644 index 0000000000000000000000000000000000000000..65d5e116cd2304b7e0835e28080b04499eea3fec --- /dev/null +++ b/third_party/tonic/tests/dart_weak_persistent_handle_unittest.cc @@ -0,0 +1,167 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/testing/dart_isolate_runner.h" +#include "flutter/testing/fixture_test.h" + +namespace flutter { +namespace testing { + +class DartWeakPersistentHandle : public FixtureTest { + public: + DartWeakPersistentHandle() + : settings_(CreateSettingsForFixture()), + vm_(DartVMRef::Create(settings_)) {} + + ~DartWeakPersistentHandle() = default; + + [[nodiscard]] bool RunWithEntrypoint(const std::string& entrypoint) { + if (running_isolate_) { + return false; + } + auto thread = CreateNewThread(); + TaskRunners single_threaded_task_runner(GetCurrentTestName(), thread, + thread, thread, thread); + auto isolate = + RunDartCodeInIsolate(vm_, settings_, single_threaded_task_runner, + entrypoint, {}, GetFixturesPath()); + if (!isolate || isolate->get()->GetPhase() != DartIsolate::Phase::Running) { + return false; + } + + running_isolate_ = std::move(isolate); + return true; + } + + [[nodiscard]] bool RunInIsolateScope(std::function closure) { + return running_isolate_->RunInIsolateScope(closure); + } + + private: + Settings settings_; + DartVMRef vm_; + std::unique_ptr running_isolate_; + FML_DISALLOW_COPY_AND_ASSIGN(DartWeakPersistentHandle); +}; + +void NopFinalizer(void* isolate_callback_data, void* peer) {} + +TEST_F(DartWeakPersistentHandle, ClearImmediately) { + auto weak_persistent_value = tonic::DartWeakPersistentValue(); + + fml::AutoResetWaitableEvent event; + + AddNativeCallback( + "GiveObjectToNative", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + auto handle = Dart_GetNativeArgument(args, 0); + + auto dart_state = tonic::DartState::Current(); + ASSERT_TRUE(dart_state); + ASSERT_TRUE(tonic::DartState::Current()); + weak_persistent_value.Set(dart_state, handle, nullptr, 0, NopFinalizer); + + weak_persistent_value.Clear(); + + event.Signal(); + })); + + ASSERT_TRUE(RunWithEntrypoint("callGiveObjectToNative")); + + event.Wait(); +} + +TEST_F(DartWeakPersistentHandle, ClearLaterCc) { + auto weak_persistent_value = tonic::DartWeakPersistentValue(); + + fml::AutoResetWaitableEvent event; + + AddNativeCallback( + "GiveObjectToNative", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + auto handle = Dart_GetNativeArgument(args, 0); + + auto dart_state = tonic::DartState::Current(); + ASSERT_TRUE(dart_state); + ASSERT_TRUE(tonic::DartState::Current()); + weak_persistent_value.Set(dart_state, handle, nullptr, 0, NopFinalizer); + + // Do not clear handle immediately. + + event.Signal(); + })); + + ASSERT_TRUE(RunWithEntrypoint("callGiveObjectToNative")); + + event.Wait(); + + ASSERT_TRUE(RunInIsolateScope([&weak_persistent_value]() -> bool { + // Clear on initiative of native. + weak_persistent_value.Clear(); + return true; + })); +} + +TEST_F(DartWeakPersistentHandle, ClearLaterDart) { + auto weak_persistent_value = tonic::DartWeakPersistentValue(); + + fml::AutoResetWaitableEvent event; + + AddNativeCallback( + "GiveObjectToNative", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + auto handle = Dart_GetNativeArgument(args, 0); + + auto dart_state = tonic::DartState::Current(); + ASSERT_TRUE(dart_state); + ASSERT_TRUE(tonic::DartState::Current()); + weak_persistent_value.Set(dart_state, handle, nullptr, 0, NopFinalizer); + + // Do not clear handle immediately. + })); + + AddNativeCallback("SignalDone", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + // Clear on initiative of Dart. + weak_persistent_value.Clear(); + + event.Signal(); + })); + + ASSERT_TRUE(RunWithEntrypoint("testClearLater")); + + event.Wait(); +} + +// Handle outside the test body scope so it survives until isolate shutdown. +tonic::DartWeakPersistentValue global_weak_persistent_value = + tonic::DartWeakPersistentValue(); + +TEST_F(DartWeakPersistentHandle, ClearOnShutdown) { + fml::AutoResetWaitableEvent event; + + AddNativeCallback("GiveObjectToNative", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + auto handle = Dart_GetNativeArgument(args, 0); + + auto dart_state = tonic::DartState::Current(); + ASSERT_TRUE(dart_state); + ASSERT_TRUE(tonic::DartState::Current()); + + // The test is repeated, ensure the global var is + // cleared before use. + global_weak_persistent_value.Clear(); + + global_weak_persistent_value.Set( + dart_state, handle, nullptr, 0, NopFinalizer); + + // Do not clear handle, so it is cleared on shutdown. + + event.Signal(); + })); + + ASSERT_TRUE(RunWithEntrypoint("callGiveObjectToNative")); + + event.Wait(); +} + +} // namespace testing +} // namespace flutter diff --git a/third_party/tonic/tests/fixtures/tonic_test.dart b/third_party/tonic/tests/fixtures/tonic_test.dart new file mode 100644 index 0000000000000000000000000000000000000000..83e92d4dc4469318d2e8d0efe3ab74ae76e8af95 --- /dev/null +++ b/third_party/tonic/tests/fixtures/tonic_test.dart @@ -0,0 +1,25 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +void main() {} + +class SomeClass { + int i; + SomeClass(this.i); +} + +void giveObjectToNative(Object someObject) native 'GiveObjectToNative'; + +void signalDone() native 'SignalDone'; + +@pragma('vm:entry-point') +void callGiveObjectToNative() { + giveObjectToNative(SomeClass(123)); +} + +@pragma('vm:entry-point') +void testClearLater() { + giveObjectToNative(SomeClass(123)); + signalDone(); +} diff --git a/third_party/tonic/typed_data/dart_byte_data.cc b/third_party/tonic/typed_data/dart_byte_data.cc index dc312de3f2f353e98bbb7eda57cb52ee10329272..cfb07399f22eb9f9f522a6f4d622532dcca60fbb 100644 --- a/third_party/tonic/typed_data/dart_byte_data.cc +++ b/third_party/tonic/typed_data/dart_byte_data.cc @@ -16,9 +16,7 @@ namespace { // with a buffer allocated outside the Dart heap. const int kExternalSizeThreshold = 1000; -void FreeFinalizer(void* isolate_callback_data, - Dart_WeakPersistentHandle handle, - void* peer) { +void FreeFinalizer(void* isolate_callback_data, void* peer) { free(peer); }