Fix data race in DartIsolateGroupData. (flutter-team-archive#15949)

This class is meant to be thread safe. In fact, its headerdoc statement on
thread safety even mentions this. All fields on the class are readonly except
the child isolate preparer. This field is set during VM instantiated isolate
initialization. The VM may launch multiple isolate in the same isolate group on
at the same time (each on a VM backed thread pool thread). Attempting to set the
field without synchronization is a data race.

Fixes flutter/flutter#49358
Fixes b/147798920

Author: chinmaygarde

runtime/dart_isolate.cc

@@ -814,7 +814,7 @@ bool DartIsolate::InitializeIsolate(
   // also a root isolate) by the utility routines in the VM. However, secondary
   // isolates will be run by the VM if they are marked as runnable.
   if (!embedder_isolate->IsRootIsolate()) {
-    const ChildIsolatePreparer& child_isolate_preparer =
+    auto child_isolate_preparer =
         embedder_isolate->GetIsolateGroupData().GetChildIsolatePreparer();
     FML_DCHECK(child_isolate_preparer);
     if (!child_isolate_preparer(embedder_isolate.get())) {

runtime/dart_isolate_group_data.cc

@@ -44,8 +44,8 @@ const std::string& DartIsolateGroupData::GetAdvisoryScriptEntrypoint() const {
   return advisory_script_entrypoint_;
 }
 
-const ChildIsolatePreparer& DartIsolateGroupData::GetChildIsolatePreparer()
-    const {
+ChildIsolatePreparer DartIsolateGroupData::GetChildIsolatePreparer() const {
+  std::scoped_lock lock(child_isolate_preparer_mutex_);
   return child_isolate_preparer_;
 }
 
@@ -59,6 +59,7 @@ const fml::closure& DartIsolateGroupData::GetIsolateShutdownCallback() const {
 
 void DartIsolateGroupData::SetChildIsolatePreparer(
     const ChildIsolatePreparer& value) {
+  std::scoped_lock lock(child_isolate_preparer_mutex_);
   child_isolate_preparer_ = value;
 }
 

runtime/dart_isolate_group_data.h

@@ -5,6 +5,7 @@
 #ifndef FLUTTER_RUNTIME_DART_ISOLATE_GROUP_DATA_H_
 #define FLUTTER_RUNTIME_DART_ISOLATE_GROUP_DATA_H_
 
+#include <mutex>
 #include <string>
 
 #include "flutter/common/settings.h"
@@ -37,25 +38,32 @@ class DartIsolateGroupData {
   ~DartIsolateGroupData();
 
   const Settings& GetSettings() const;
+
   fml::RefPtr<const DartSnapshot> GetIsolateSnapshot() const;
+
   const std::string& GetAdvisoryScriptURI() const;
+
   const std::string& GetAdvisoryScriptEntrypoint() const;
-  const ChildIsolatePreparer& GetChildIsolatePreparer() const;
+
+  ChildIsolatePreparer GetChildIsolatePreparer() const;
+
   const fml::closure& GetIsolateCreateCallback() const;
+
   const fml::closure& GetIsolateShutdownCallback() const;
 
   void SetChildIsolatePreparer(const ChildIsolatePreparer& value);
 
  private:
-  FML_DISALLOW_COPY_AND_ASSIGN(DartIsolateGroupData);
-
   const Settings settings_;
   const fml::RefPtr<const DartSnapshot> isolate_snapshot_;
   const std::string advisory_script_uri_;
   const std::string advisory_script_entrypoint_;
+  mutable std::mutex child_isolate_preparer_mutex_;
   ChildIsolatePreparer child_isolate_preparer_;
   const fml::closure isolate_create_callback_;
   const fml::closure isolate_shutdown_callback_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(DartIsolateGroupData);
 };
 
 }  // namespace flutter