Avoid leaking the VM in runtime_unittests and update failing tests. (flutter#8626)

chinmaygarde · web-flow · commit 7d3caf895255 · 2019-04-17T19:06:03.000-07:00
The failing tests were depending on the old assumption that the VM would never
shutdown.
diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc
@@ -21,6 +21,7 @@ namespace testing {
 using DartIsolateTest = RuntimeTest;
 
 TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) {
+  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
   auto settings = CreateSettingsForFixture();
   auto vm_ref = DartVMRef::Create(settings);
   ASSERT_TRUE(vm_ref);
@@ -50,6 +51,7 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) {
 }
 
 TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) {
+  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
   auto settings = CreateSettingsForFixture();
   auto vm_ref = DartVMRef::Create(settings);
   ASSERT_TRUE(vm_ref);
@@ -143,12 +145,12 @@ class AutoIsolateShutdown {
   FML_DISALLOW_COPY_AND_ASSIGN(AutoIsolateShutdown);
 };
 
-static void RunDartCodeInIsolate(std::unique_ptr<AutoIsolateShutdown>& result,
+static void RunDartCodeInIsolate(DartVMRef& vm_ref,
+                                 std::unique_ptr<AutoIsolateShutdown>& result,
                                  const Settings& settings,
                                  fml::RefPtr<fml::TaskRunner> task_runner,
                                  std::string entrypoint) {
   FML_CHECK(task_runner->RunsTasksOnCurrentThread());
-  auto vm_ref = DartVMRef::Create(settings);
 
   if (!vm_ref) {
     return;
@@ -249,36 +251,46 @@ static void RunDartCodeInIsolate(std::unique_ptr<AutoIsolateShutdown>& result,
 }
 
 static std::unique_ptr<AutoIsolateShutdown> RunDartCodeInIsolate(
+    DartVMRef& vm_ref,
     const Settings& settings,
     fml::RefPtr<fml::TaskRunner> task_runner,
     std::string entrypoint) {
   std::unique_ptr<AutoIsolateShutdown> result;
   fml::AutoResetWaitableEvent latch;
   fml::TaskRunner::RunNowOrPostTask(
       task_runner, fml::MakeCopyable([&]() mutable {
-        RunDartCodeInIsolate(result, settings, task_runner, entrypoint);
+        RunDartCodeInIsolate(vm_ref, result, settings, task_runner, entrypoint);
         latch.Signal();
       }));
   latch.Wait();
   return result;
 }
 
 TEST_F(DartIsolateTest, IsolateCanLoadAndRunDartCode) {
-  auto isolate = RunDartCodeInIsolate(CreateSettingsForFixture(),
-                                      GetCurrentTaskRunner(), "main");
+  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
+  const auto settings = CreateSettingsForFixture();
+  auto vm_ref = DartVMRef::Create(settings);
+  auto isolate =
+      RunDartCodeInIsolate(vm_ref, settings, GetCurrentTaskRunner(), "main");
   ASSERT_TRUE(isolate);
   ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
 }
 
 TEST_F(DartIsolateTest, IsolateCannotLoadAndRunUnknownDartEntrypoint) {
-  auto isolate = RunDartCodeInIsolate(
-      CreateSettingsForFixture(), GetCurrentTaskRunner(), "thisShouldNotExist");
+  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
+  const auto settings = CreateSettingsForFixture();
+  auto vm_ref = DartVMRef::Create(settings);
+  auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetCurrentTaskRunner(),
+                                      "thisShouldNotExist");
   ASSERT_FALSE(isolate);
 }
 
 TEST_F(DartIsolateTest, CanRunDartCodeCodeSynchronously) {
-  auto isolate = RunDartCodeInIsolate(CreateSettingsForFixture(),
-                                      GetCurrentTaskRunner(), "main");
+  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
+  const auto settings = CreateSettingsForFixture();
+  auto vm_ref = DartVMRef::Create(settings);
+  auto isolate =
+      RunDartCodeInIsolate(vm_ref, settings, GetCurrentTaskRunner(), "main");
 
   ASSERT_TRUE(isolate);
   ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
@@ -292,15 +304,17 @@ TEST_F(DartIsolateTest, CanRunDartCodeCodeSynchronously) {
 }
 
 TEST_F(DartIsolateTest, CanRegisterNativeCallback) {
+  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
   fml::AutoResetWaitableEvent latch;
   AddNativeCallback("NotifyNative",
                     CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
                       FML_LOG(ERROR) << "Hello from Dart!";
                       latch.Signal();
                     })));
-  auto isolate =
-      RunDartCodeInIsolate(CreateSettingsForFixture(), GetThreadTaskRunner(),
-                           "canRegisterNativeCallback");
+  const auto settings = CreateSettingsForFixture();
+  auto vm_ref = DartVMRef::Create(settings);
+  auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetThreadTaskRunner(),
+                                      "canRegisterNativeCallback");
   ASSERT_TRUE(isolate);
   ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
   latch.Wait();
diff --git a/runtime/dart_vm_unittests.cc b/runtime/dart_vm_unittests.cc
@@ -4,23 +4,23 @@
 
 #include "flutter/runtime/dart_vm.h"
 #include "flutter/runtime/dart_vm_lifecycle.h"
+#include "flutter/runtime/runtime_test.h"
 #include "gtest/gtest.h"
 
 namespace flutter {
+namespace testing {
 
-TEST(DartVM, SimpleInitialization) {
-  Settings settings = {};
-  settings.task_observer_add = [](intptr_t, fml::closure) {};
-  settings.task_observer_remove = [](intptr_t) {};
-  auto vm = DartVMRef::Create(settings);
+using DartVMTest = RuntimeTest;
+
+TEST_F(DartVMTest, SimpleInitialization) {
+  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
+  auto vm = DartVMRef::Create(CreateSettingsForFixture());
   ASSERT_TRUE(vm);
 }
 
-TEST(DartVM, SimpleIsolateNameServer) {
-  Settings settings = {};
-  settings.task_observer_add = [](intptr_t, fml::closure) {};
-  settings.task_observer_remove = [](intptr_t) {};
-  auto vm = DartVMRef::Create(settings);
+TEST_F(DartVMTest, SimpleIsolateNameServer) {
+  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
+  auto vm = DartVMRef::Create(CreateSettingsForFixture());
   ASSERT_TRUE(vm);
   ASSERT_TRUE(vm.GetVMData());
   auto ns = vm->GetIsolateNameServer();
@@ -32,4 +32,5 @@ TEST(DartVM, SimpleIsolateNameServer) {
   ASSERT_TRUE(ns->RemoveIsolateNameMapping("foobar"));
 }
 
+}  // namespace testing
 }  // namespace flutter
diff --git a/runtime/runtime_test.cc b/runtime/runtime_test.cc
@@ -80,6 +80,7 @@ void RuntimeTest::SetSnapshotsAndAssets(Settings& settings) {
 
 Settings RuntimeTest::CreateSettingsForFixture() {
   Settings settings;
+  settings.leak_vm = false;
   settings.task_observer_add = [](intptr_t, fml::closure) {};
   settings.task_observer_remove = [](intptr_t) {};
   settings.root_isolate_create_callback = [this]() {
