Avoid calling Dart timeline APIs during Dart_Cleanup (#24007)

The Dart timeline is not thread safe if an engine thread that is not
controlled by Dart calls Dart_TimelineEvent while another thread is
calling Dart_Cleanup.

Fixes https://github.com/flutter/flutter/issues/74134

ci/licenses_golden/licenses_flutter

@@ -597,6 +597,8 @@ FILE: ../../../flutter/runtime/dart_vm.cc
 FILE: ../../../flutter/runtime/dart_vm.h
 FILE: ../../../flutter/runtime/dart_vm_data.cc
 FILE: ../../../flutter/runtime/dart_vm_data.h
+FILE: ../../../flutter/runtime/dart_vm_initializer.cc
+FILE: ../../../flutter/runtime/dart_vm_initializer.h
 FILE: ../../../flutter/runtime/dart_vm_lifecycle.cc
 FILE: ../../../flutter/runtime/dart_vm_lifecycle.h
 FILE: ../../../flutter/runtime/dart_vm_unittests.cc

fml/trace_event.cc

@@ -19,6 +19,7 @@ namespace tracing {
 
 namespace {
 AsciiTrie gAllowlist;
+TimelineEventHandler gTimelineEventHandler;
 
 inline void FlutterTimelineEvent(const char* label,
                                  int64_t timestamp0,
@@ -27,9 +28,9 @@ inline void FlutterTimelineEvent(const char* label,
                                  intptr_t argument_count,
                                  const char** argument_names,
                                  const char** argument_values) {
-  if (gAllowlist.Query(label)) {
-    Dart_TimelineEvent(label, timestamp0, timestamp1_or_async_id, type,
-                       argument_count, argument_names, argument_values);
+  if (gTimelineEventHandler && gAllowlist.Query(label)) {
+    gTimelineEventHandler(label, timestamp0, timestamp1_or_async_id, type,
+                          argument_count, argument_names, argument_values);
   }
 }
 }  // namespace
@@ -38,6 +39,10 @@ void TraceSetAllowlist(const std::vector<std::string>& allowlist) {
   gAllowlist.Fill(allowlist);
 }
 
+void TraceSetTimelineEventHandler(TimelineEventHandler handler) {
+  gTimelineEventHandler = handler;
+}
+
 size_t TraceNonce() {
   static std::atomic_size_t gLastItem;
   return ++gLastItem;
@@ -288,6 +293,8 @@ void TraceEventFlowEnd0(TraceArg category_group, TraceArg name, TraceIDArg id) {
 
 void TraceSetAllowlist(const std::vector<std::string>& allowlist) {}
 
+void TraceSetTimelineEventHandler(TimelineEventHandler handler) {}
+
 size_t TraceNonce() {
   return 0;
 }

fml/trace_event.h

@@ -5,6 +5,8 @@
 #ifndef FLUTTER_FML_TRACE_EVENT_H_
 #define FLUTTER_FML_TRACE_EVENT_H_
 
+#include <functional>
+
 #include "flutter/fml/build_config.h"
 
 #if defined(OS_FUCHSIA)
@@ -145,6 +147,16 @@ using TraceIDArg = int64_t;
 
 void TraceSetAllowlist(const std::vector<std::string>& allowlist);
 
+using TimelineEventHandler = std::function<void(const char*,
+                                                int64_t,
+                                                int64_t,
+                                                Dart_Timeline_Event_Type,
+                                                intptr_t,
+                                                const char**,
+                                                const char**)>;
+
+void TraceSetTimelineEventHandler(TimelineEventHandler handler);
+
 void TraceTimelineEvent(TraceArg category_group,
                         TraceArg name,
                         int64_t timestamp_micros,

runtime/BUILD.gn

@@ -49,6 +49,8 @@ source_set("runtime") {
     "dart_vm.h",
     "dart_vm_data.cc",
     "dart_vm_data.h",
+    "dart_vm_initializer.cc",
+    "dart_vm_initializer.h",
     "dart_vm_lifecycle.cc",
     "dart_vm_lifecycle.h",
     "embedder_resources.cc",

runtime/dart_vm.cc

@@ -24,6 +24,7 @@
 #include "flutter/lib/ui/dart_ui.h"
 #include "flutter/runtime/dart_isolate.h"
 #include "flutter/runtime/dart_service_isolate.h"
+#include "flutter/runtime/dart_vm_initializer.h"
 #include "flutter/runtime/ptrace_check.h"
 #include "third_party/dart/runtime/include/bin/dart_io_api.h"
 #include "third_party/skia/include/core/SkExecutor.h"
@@ -437,11 +438,7 @@ DartVM::DartVM(std::shared_ptr<const DartVMData> vm_data,
     params.thread_exit = ThreadExitCallback;
     params.get_service_assets = GetVMServiceAssetsArchiveCallback;
     params.entropy_source = dart::bin::GetEntropy;
-    char* init_error = Dart_Initialize(&params);
-    if (init_error) {
-      FML_LOG(FATAL) << "Error while initializing the Dart VM: " << init_error;
-      ::free(init_error);
-    }
+    DartVMInitializer::Initialize(&params);
     // Send the earliest available timestamp in the application lifecycle to
     // timeline. The difference between this timestamp and the time we render
     // the very first frame gives us a good idea about Flutter's startup time.
@@ -486,14 +483,9 @@ DartVM::~DartVM() {
     Dart_ExitIsolate();
   }
 
-  char* result = Dart_Cleanup();
+  DartVMInitializer::Cleanup();
 
   dart::bin::CleanupDartIo();
-
-  FML_CHECK(result == nullptr)
-      << "Could not cleanly shut down the Dart VM. Error: \"" << result
-      << "\".";
-  free(result);
 }
 
 std::shared_ptr<const DartVMData> DartVM::GetVMData() const {

runtime/dart_vm_initializer.cc

@@ -0,0 +1,71 @@
+// 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/runtime/dart_vm_initializer.h"
+
+#include <atomic>
+
+#include "flutter/fml/logging.h"
+#include "flutter/fml/synchronization/shared_mutex.h"
+#include "flutter/fml/trace_event.h"
+
+// Tracks whether Dart has been initialized and if it is safe to call Dart
+// APIs.
+static std::atomic<bool> gDartInitialized;
+
+void DartVMInitializer::Initialize(Dart_InitializeParams* params) {
+  FML_DCHECK(!gDartInitialized);
+
+  char* error = Dart_Initialize(params);
+  if (error) {
+    FML_LOG(FATAL) << "Error while initializing the Dart VM: " << error;
+    ::free(error);
+  } else {
+    gDartInitialized = true;
+  }
+
+  fml::tracing::TraceSetTimelineEventHandler(LogDartTimelineEvent);
+}
+
+void DartVMInitializer::Cleanup() {
+  FML_DCHECK(gDartInitialized);
+
+  // Dart_TimelineEvent is unsafe during a concurrent call to Dart_Cleanup
+  // because Dart_Cleanup will destroy the timeline recorder.  Clear the
+  // initialized flag so that future calls to LogDartTimelineEvent will not
+  // call Dart_TimelineEvent.
+  //
+  // Note that this is inherently racy.  If a thread sees that gDartInitialized
+  // is set and proceeds to call Dart_TimelineEvent shortly before another
+  // thread calls Dart_Cleanup, then the Dart_TimelineEvent call may crash
+  // if Dart_Cleanup deletes the timeline before Dart_TimelineEvent completes.
+  // In practice this is unlikely because Dart_Cleanup does significant other
+  // work before deleting the timeline.
+  //
+  // The engine can not safely guard Dart_Cleanup and LogDartTimelineEvent with
+  // a lock due to the risk of deadlocks.  Dart_Cleanup waits for various
+  // Dart-owned threads to shut down.  If one of those threads invokes an engine
+  // callback that calls LogDartTimelineEvent while the Dart_Cleanup thread owns
+  // the lock, then Dart_Cleanup would deadlock.
+  gDartInitialized = false;
+
+  char* error = Dart_Cleanup();
+  if (error) {
+    FML_LOG(FATAL) << "Error while cleaning up the Dart VM: " << error;
+    ::free(error);
+  }
+}
+
+void DartVMInitializer::LogDartTimelineEvent(const char* label,
+                                             int64_t timestamp0,
+                                             int64_t timestamp1_or_async_id,
+                                             Dart_Timeline_Event_Type type,
+                                             intptr_t argument_count,
+                                             const char** argument_names,
+                                             const char** argument_values) {
+  if (gDartInitialized) {
+    Dart_TimelineEvent(label, timestamp0, timestamp1_or_async_id, type,
+                       argument_count, argument_names, argument_values);
+  }
+}

runtime/dart_vm_initializer.h

@@ -0,0 +1,26 @@
+// 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 FLUTTER_RUNTIME_DART_VM_INITIALIZER_H_
+#define FLUTTER_RUNTIME_DART_VM_INITIALIZER_H_
+
+#include "third_party/dart/runtime/include/dart_api.h"
+#include "third_party/dart/runtime/include/dart_tools_api.h"
+
+class DartVMInitializer {
+ public:
+  static void Initialize(Dart_InitializeParams* params);
+  static void Cleanup();
+
+ private:
+  static void LogDartTimelineEvent(const char* label,
+                                   int64_t timestamp0,
+                                   int64_t timestamp1_or_async_id,
+                                   Dart_Timeline_Event_Type type,
+                                   intptr_t argument_count,
+                                   const char** argument_names,
+                                   const char** argument_values);
+};
+
+#endif  // FLUTTER_RUNTIME_DART_VM_INITIALIZER_H_

runtime/dart_vm_lifecycle.h

@@ -11,6 +11,7 @@
 #include "flutter/lib/ui/isolate_name_server/isolate_name_server.h"
 #include "flutter/runtime/dart_vm.h"
 #include "flutter/runtime/service_protocol.h"
+#include "third_party/dart/runtime/include/dart_tools_api.h"
 
 namespace flutter {