diff --git a/runtime/BUILD.gn b/runtime/BUILD.gn index 5df979ed689a9..d85c3833ce5c8 100644 --- a/runtime/BUILD.gn +++ b/runtime/BUILD.gn @@ -94,6 +94,7 @@ executable("runtime_unittests") { ":runtime", ":runtime_fixtures", "$flutter_root/fml", + "$flutter_root/shell/common", "$flutter_root/lib/snapshot", "$flutter_root/testing", "//third_party/dart/runtime:libdart_jit", diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index d99e1bfcdbdbb..05f4f778bc46b 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -8,11 +8,6 @@ #include "flutter/testing/testing.h" #include "flutter/testing/thread_test.h" -#define CURRENT_TEST_NAME \ - std::string { \ - ::testing::UnitTest::GetInstance()->current_test_info()->name() \ - } - namespace blink { using DartIsolateTest = ::testing::ThreadTest; @@ -23,11 +18,11 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) { settings.task_observer_remove = [](intptr_t) {}; auto vm = DartVM::ForProcess(settings); ASSERT_TRUE(vm); - TaskRunners task_runners(CURRENT_TEST_NAME, // - GetCurrentTaskRunner(), // - GetCurrentTaskRunner(), // - GetCurrentTaskRunner(), // - GetCurrentTaskRunner() // + TaskRunners task_runners(testing::GetCurrentTestName(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner() // ); auto weak_isolate = DartIsolate::CreateRootIsolate( vm.get(), // vm @@ -53,11 +48,11 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) { settings.task_observer_remove = [](intptr_t) {}; auto vm = DartVM::ForProcess(settings); ASSERT_TRUE(vm); - TaskRunners task_runners(CURRENT_TEST_NAME, // - GetCurrentTaskRunner(), // - GetCurrentTaskRunner(), // - GetCurrentTaskRunner(), // - GetCurrentTaskRunner() // + TaskRunners task_runners(testing::GetCurrentTestName(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner() // ); auto weak_isolate = DartIsolate::CreateRootIsolate( vm.get(), // vm diff --git a/runtime/dart_vm.cc b/runtime/dart_vm.cc index 76613f6e4aa47..0e28c057a2501 100644 --- a/runtime/dart_vm.cc +++ b/runtime/dart_vm.cc @@ -15,6 +15,7 @@ #include "flutter/fml/file.h" #include "flutter/fml/logging.h" #include "flutter/fml/mapping.h" +#include "flutter/fml/synchronization/thread_annotations.h" #include "flutter/fml/time/time_delta.h" #include "flutter/fml/trace_event.h" #include "flutter/lib/io/dart_io.h" @@ -233,54 +234,63 @@ static void EmbedderInformationCallback(Dart_EmbedderInformation* info) { info->name = "Flutter"; } -fml::RefPtr DartVM::ForProcess(Settings settings) { +std::shared_ptr DartVM::ForProcess(Settings settings) { return ForProcess(settings, nullptr, nullptr, nullptr); } -static std::once_flag gVMInitialization; static std::mutex gVMMutex; -static fml::RefPtr gVM; +static std::weak_ptr gVM; -fml::RefPtr DartVM::ForProcess( +std::shared_ptr DartVM::ForProcess( Settings settings, fml::RefPtr vm_snapshot, fml::RefPtr isolate_snapshot, fml::RefPtr shared_snapshot) { std::lock_guard lock(gVMMutex); - std::call_once(gVMInitialization, [settings, // - vm_snapshot, // - isolate_snapshot, // - shared_snapshot // - ]() mutable { - if (!vm_snapshot) { - vm_snapshot = DartSnapshot::VMSnapshotFromSettings(settings); - } - if (!(vm_snapshot && vm_snapshot->IsValid())) { - FML_LOG(ERROR) << "VM snapshot must be valid."; - return; - } - if (!isolate_snapshot) { - isolate_snapshot = DartSnapshot::IsolateSnapshotFromSettings(settings); - } - if (!(isolate_snapshot && isolate_snapshot->IsValid())) { - FML_LOG(ERROR) << "Isolate snapshot must be valid."; - return; - } - if (!shared_snapshot) { - shared_snapshot = DartSnapshot::Empty(); - } - gVM = fml::MakeRefCounted(settings, // - std::move(vm_snapshot), // - std::move(isolate_snapshot), // - std::move(shared_snapshot) // - ); - }); - return gVM; + + // TODO(chinmaygarde): Make this an assertion instead so that callers are + // notified that the new settings might not hold because an exiting VM was + // being reused. + if (auto vm = gVM.lock()) { + return vm; + } + + if (!vm_snapshot) { + vm_snapshot = DartSnapshot::VMSnapshotFromSettings(settings); + } + + if (!(vm_snapshot && vm_snapshot->IsValid())) { + FML_LOG(ERROR) << "VM snapshot must be valid."; + return {}; + } + + if (!isolate_snapshot) { + isolate_snapshot = DartSnapshot::IsolateSnapshotFromSettings(settings); + } + + if (!(isolate_snapshot && isolate_snapshot->IsValid())) { + FML_LOG(ERROR) << "Isolate snapshot must be valid."; + return {}; + } + + if (!shared_snapshot) { + shared_snapshot = DartSnapshot::Empty(); + } + + // Note: std::make_shared unviable due to hidden constructor. + auto vm = std::shared_ptr(new DartVM(settings, // + std::move(vm_snapshot), // + std::move(isolate_snapshot), // + std::move(shared_snapshot) // + )); + + gVM = vm; + + return vm; } -fml::RefPtr DartVM::ForProcessIfInitialized() { - std::lock_guard lock(gVMMutex); - return gVM; +std::shared_ptr DartVM::ForProcessIfInitialized() { + return gVM.lock(); } DartVM::DartVM(const Settings& settings, @@ -290,8 +300,7 @@ DartVM::DartVM(const Settings& settings, : settings_(settings), vm_snapshot_(std::move(vm_snapshot)), isolate_snapshot_(std::move(isolate_snapshot)), - shared_snapshot_(std::move(shared_snapshot)), - weak_factory_(this) { + shared_snapshot_(std::move(shared_snapshot)) { TRACE_EVENT0("flutter", "DartVMInitializer"); FML_DLOG(INFO) << "Attempting Dart VM launch for mode: " << (IsRunningPrecompiledCode() ? "AOT" : "Interpreter"); @@ -381,8 +390,6 @@ DartVM::DartVM(const Settings& settings, DartUI::InitForGlobal(); - Dart_SetFileModifiedCallback(&DartFileModifiedCallback); - { TRACE_EVENT0("flutter", "Dart_Initialize"); Dart_InitializeParams params = {}; @@ -420,6 +427,8 @@ DartVM::DartVM(const Settings& settings, } } + Dart_SetFileModifiedCallback(&DartFileModifiedCallback); + // Allow streaming of stdout and stderr by the Dart vm. Dart_SetServiceStreamCallbacks(&ServiceStreamListenCallback, &ServiceStreamCancelCallback); @@ -428,10 +437,15 @@ DartVM::DartVM(const Settings& settings, } DartVM::~DartVM() { + std::lock_guard lock(gVMMutex); if (Dart_CurrentIsolate() != nullptr) { Dart_ExitIsolate(); } + char* result = Dart_Cleanup(); + + dart::bin::CleanupDartIo(); + if (result != nullptr) { FML_LOG(ERROR) << "Could not cleanly shut down the Dart VM. Message: \"" << result << "\"."; @@ -463,8 +477,4 @@ ServiceProtocol& DartVM::GetServiceProtocol() { return service_protocol_; } -fml::WeakPtr DartVM::GetWeakPtr() { - return weak_factory_.GetWeakPtr(); -} - } // namespace blink diff --git a/runtime/dart_vm.h b/runtime/dart_vm.h index 9a1bde888dd86..be7196e5f5dd8 100644 --- a/runtime/dart_vm.h +++ b/runtime/dart_vm.h @@ -6,6 +6,7 @@ #define FLUTTER_RUNTIME_DART_VM_H_ #include +#include #include #include @@ -24,17 +25,19 @@ namespace blink { -class DartVM : public fml::RefCountedThreadSafe { +class DartVM { public: - static fml::RefPtr ForProcess(Settings settings); + ~DartVM(); + + static std::shared_ptr ForProcess(Settings settings); - static fml::RefPtr ForProcess( + static std::shared_ptr ForProcess( Settings settings, fml::RefPtr vm_snapshot, fml::RefPtr isolate_snapshot, fml::RefPtr shared_snapshot); - static fml::RefPtr ForProcessIfInitialized(); + static std::shared_ptr ForProcessIfInitialized(); static bool IsRunningPrecompiledCode(); @@ -50,8 +53,6 @@ class DartVM : public fml::RefCountedThreadSafe { fml::RefPtr GetSharedSnapshot() const; - fml::WeakPtr GetWeakPtr(); - ServiceProtocol& GetServiceProtocol(); private: @@ -61,17 +62,12 @@ class DartVM : public fml::RefCountedThreadSafe { const fml::RefPtr isolate_snapshot_; const fml::RefPtr shared_snapshot_; ServiceProtocol service_protocol_; - fml::WeakPtrFactory weak_factory_; DartVM(const Settings& settings, fml::RefPtr vm_snapshot, fml::RefPtr isolate_snapshot, fml::RefPtr shared_snapshot); - ~DartVM(); - - FML_FRIEND_REF_COUNTED_THREAD_SAFE(DartVM); - FML_FRIEND_MAKE_REF_COUNTED(DartVM); FML_DISALLOW_COPY_AND_ASSIGN(DartVM); }; diff --git a/runtime/dart_vm_unittests.cc b/runtime/dart_vm_unittests.cc index 21e3d3d3fa58f..72d1908a65a3d 100644 --- a/runtime/dart_vm_unittests.cc +++ b/runtime/dart_vm_unittests.cc @@ -3,25 +3,29 @@ // found in the LICENSE file. #include "flutter/runtime/dart_vm.h" +#include "flutter/shell/common/thread_host.h" +#include "flutter/testing/testing.h" +#include "flutter/testing/thread_test.h" #include "gtest/gtest.h" namespace blink { -TEST(DartVM, SimpleInitialization) { - Settings settings = {}; +static Settings GetTestSettings() { + Settings settings; settings.task_observer_add = [](intptr_t, fml::closure) {}; settings.task_observer_remove = [](intptr_t) {}; - auto vm = DartVM::ForProcess(settings); + return settings; +} + +TEST(DartVM, SimpleInitialization) { + auto vm = DartVM::ForProcess(GetTestSettings()); ASSERT_TRUE(vm); - ASSERT_EQ(vm, DartVM::ForProcess(settings)); + ASSERT_EQ(vm, DartVM::ForProcess(GetTestSettings())); ASSERT_FALSE(DartVM::IsRunningPrecompiledCode()); } TEST(DartVM, SimpleIsolateNameServer) { - Settings settings = {}; - settings.task_observer_add = [](intptr_t, fml::closure) {}; - settings.task_observer_remove = [](intptr_t) {}; - auto vm = DartVM::ForProcess(settings); + auto vm = DartVM::ForProcess(GetTestSettings()); auto ns = vm->GetIsolateNameServer(); ASSERT_EQ(ns->LookupIsolatePortByName("foobar"), ILLEGAL_PORT); ASSERT_FALSE(ns->RemoveIsolateNameMapping("foobar")); @@ -31,4 +35,64 @@ TEST(DartVM, SimpleIsolateNameServer) { ASSERT_TRUE(ns->RemoveIsolateNameMapping("foobar")); } +TEST(DartVM, CanReinitializeVMOverAndOver) { + for (size_t i = 0; i < 1000; ++i) { + FML_LOG(INFO) << "Run " << i + 1; + // VM should not already be running. + ASSERT_FALSE(DartVM::ForProcessIfInitialized()); + auto vm = DartVM::ForProcess(GetTestSettings()); + ASSERT_TRUE(vm); + ASSERT_TRUE(DartVM::ForProcessIfInitialized()); + } +} + +using DartVMThreadTest = ::testing::ThreadTest; + +TEST_F(DartVMThreadTest, CanRunIsolatesInANewVM) { + for (size_t i = 0; i < 1000; ++i) { + FML_LOG(INFO) << "Run " << i + 1; + // VM should not already be running. + ASSERT_FALSE(DartVM::ForProcessIfInitialized()); + auto vm = DartVM::ForProcess(GetTestSettings()); + ASSERT_TRUE(vm); + ASSERT_TRUE(DartVM::ForProcessIfInitialized()); + + Settings settings = {}; + + settings.task_observer_add = [](intptr_t, fml::closure) {}; + settings.task_observer_remove = [](intptr_t) {}; + + auto labels = testing::GetCurrentTestName() + std::to_string(i); + shell::ThreadHost host(labels, shell::ThreadHost::Type::UI | + shell::ThreadHost::Type::GPU | + shell::ThreadHost::Type::IO); + + TaskRunners task_runners( + labels, // task runner labels + GetCurrentTaskRunner(), // platform task runner + host.gpu_thread->GetTaskRunner(), // GPU task runner + host.ui_thread->GetTaskRunner(), // UI task runner + host.io_thread->GetTaskRunner() // IO task runner + ); + + auto weak_isolate = DartIsolate::CreateRootIsolate( + vm.get(), // vm + vm->GetIsolateSnapshot(), // isolate snapshot + vm->GetSharedSnapshot(), // shared snapshot + std::move(task_runners), // task runners + nullptr, // window + {}, // snapshot delegate + {}, // resource context + nullptr, // unref qeueue + "main.dart", // advisory uri + "main" // advisory entrypoint + ); + + auto root_isolate = weak_isolate.lock(); + ASSERT_TRUE(root_isolate); + ASSERT_EQ(root_isolate->GetPhase(), DartIsolate::Phase::LibrariesSetup); + ASSERT_TRUE(root_isolate->Shutdown()); + } +} + } // namespace blink diff --git a/shell/common/shell.h b/shell/common/shell.h index 2222fa45367c9..658935f620c2f 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -85,7 +85,7 @@ class Shell final : public PlatformView::Delegate, const blink::TaskRunners task_runners_; const blink::Settings settings_; - fml::RefPtr vm_; + std::shared_ptr vm_; std::unique_ptr platform_view_; // on platform task runner std::unique_ptr engine_; // on UI task runner std::unique_ptr rasterizer_; // on GPU task runner diff --git a/testing/testing.cc b/testing/testing.cc index 93e135e7ab43d..c738219541452 100644 --- a/testing/testing.cc +++ b/testing/testing.cc @@ -6,6 +6,8 @@ namespace testing { -// +std::string GetCurrentTestName() { + return UnitTest::GetInstance()->current_test_info()->name(); +} } // namespace testing diff --git a/testing/testing.h b/testing/testing.h index dac2d229b9169..0662055fea502 100644 --- a/testing/testing.h +++ b/testing/testing.h @@ -5,6 +5,8 @@ #ifndef TESTING_TESTING_H_ #define TESTING_TESTING_H_ +#include + #include "gtest/gtest.h" namespace testing { @@ -14,6 +16,8 @@ namespace testing { // error. const char* GetFixturesPath(); +std::string GetCurrentTestName(); + } // namespace testing #endif // TESTING_TESTING_H_