Second attempt to reland "[vm/concurrency] Introduce concept of Isolate Groups"

This reverts commit 3d14b75.

An Isolate Group (IG) is a collection of isolates which were spawned from the
same source. This allows the VM to:

  * have a guarantee that all isolates within one IG can safely exchange
    structured objects (currently we rely on embedder for this
    guarantee)

  * hot-reload all isolates together (currently we only reload one
    isolate, leaving same-source isolates in inconsistent state)

  * make a shared heap for all isolates from the same IG, which paves
    the way for faster communication and sharing of immutable objects.

All isolates within one IG will share the same IsolateGroupSource.

**Embedder changes**

This change makes breaking embedder API changes to support this new
concept of Isolate Groups: The existing isolate lifecycle callbacks
given to Dart_Initialize will become Isolate Group lifecycle callbacks.
A new callback `initialize_isolate` callback will be added which can
initialize a new isolate within an existing IG.

Existing embedders can be updated by performing the following renames

  Dart_CreateIsolate -> Dart_CreateIsolateGroup
  Dart_IsolateCreateCallback -> Dart_IsolateGroupCreateCallback
  Dart_IsolateCleanupCallback -> Dart_IsolateGroupShutdownCallback
  Dart_CreateIsolateFromKernel -> Dart_CreateIsolateGroupFromKernel
  Dart_CurrentIsolateData -> Dart_CurrentIsolateGroupData
  Dart_IsolateData -> Dart_IsolateGroupData
  Dart_GetNativeIsolateData -> Dart_GetNativeIsolateGroupData
  Dart_InitializeParams.create -> Dart_InitializeParams.create_group
  Dart_InitializeParams.cleanup -> Dart_InitializeParams.shutdown_group
  Dart_InitializeParams.shutdown -> Dart_InitializeParams.shutdown_isolate

By default `Isolate.spawn` will cause the creation of a new IG.

Though an embedder can opt-into supporting multiple isolates within one IG by
providing a callback to the newly added `Dart_InitializeParams.initialize_isolate`.
The responsibility of this new callback is to initialize an existing
isolate (which was setup by re-using source code from the spawning
isolate - i.e. the one which used `Isolate.spawn`) by setting native
resolvers, initializing global state, etc.

Issue #36648
Issue #36097

Original review: https://dart-review.googlesource.com/c/sdk/+/105241

Difference to original review:

  * Give each isolate it's own [Loader] (for now)
  * Sort classes during initialization for spawned isolates if app-jit is used (to match main isolate)
  * Fix IsolateData memory leak if isolate startup fails

Difference to first reland(Patchset 2):

  * Fix typo where memory was freed twice.

Change-Id: Ib1c83fe83b629cd50ae6af90ee99fdd44da882d4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108367
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>

Author: aam

runtime/bin/dart_embedder_api_impl.cc

@@ -45,9 +45,9 @@ Dart_Isolate CreateKernelServiceIsolate(const IsolateCreationData& data,
                                         const uint8_t* buffer,
                                         intptr_t buffer_size,
                                         char** error) {
-  Dart_Isolate kernel_isolate = Dart_CreateIsolateFromKernel(
+  Dart_Isolate kernel_isolate = Dart_CreateIsolateGroupFromKernel(
       data.script_uri, data.main, buffer, buffer_size, data.flags,
-      data.callback_data, error);
+      data.isolate_group_data, data.isolate_data, error);
   if (kernel_isolate == nullptr) {
     return nullptr;
   }
@@ -78,9 +78,9 @@ Dart_Isolate CreateVmServiceIsolate(const IsolateCreationData& data,
   }
   data.flags->load_vmservice_library = true;
 
-  Dart_Isolate service_isolate = Dart_CreateIsolateFromKernel(
+  Dart_Isolate service_isolate = Dart_CreateIsolateGroupFromKernel(
       data.script_uri, data.main, kernel_buffer, kernel_buffer_size, data.flags,
-      data.callback_data, error);
+      data.isolate_group_data, data.isolate_data, error);
   if (service_isolate == nullptr) {
     return nullptr;
   }

runtime/bin/gen_snapshot.cc

@@ -745,23 +745,25 @@ static int CreateIsolateAndSnapshot(const CommandLineOptions& inputs) {
     isolate_flags.entry_points = no_entry_points;
   }
 
-  IsolateData* isolate_data = new IsolateData(NULL, NULL, NULL, NULL);
+  auto isolate_group_data =
+      new IsolateGroupData(nullptr, nullptr, nullptr, nullptr, false);
   Dart_Isolate isolate;
   char* error = NULL;
   if (isolate_snapshot_data == NULL) {
     // We need to capture the vmservice library in the core snapshot, so load it
     // in the main isolate as well.
     isolate_flags.load_vmservice_library = true;
-    isolate = Dart_CreateIsolateFromKernel(NULL, NULL, kernel_buffer,
-                                           kernel_buffer_size, &isolate_flags,
-                                           isolate_data, &error);
+    isolate = Dart_CreateIsolateGroupFromKernel(
+        NULL, NULL, kernel_buffer, kernel_buffer_size, &isolate_flags,
+        isolate_group_data, /*isolate_data=*/nullptr, &error);
   } else {
-    isolate = Dart_CreateIsolate(NULL, NULL, isolate_snapshot_data,
-                                 isolate_snapshot_instructions, NULL, NULL,
-                                 &isolate_flags, isolate_data, &error);
+    isolate = Dart_CreateIsolateGroup(NULL, NULL, isolate_snapshot_data,
+                                      isolate_snapshot_instructions, NULL, NULL,
+                                      &isolate_flags, isolate_group_data,
+                                      /*isolate_data=*/nullptr, &error);
   }
   if (isolate == NULL) {
-    delete isolate_data;
+    delete isolate_group_data;
     Syslog::PrintErr("%s\n", error);
     free(error);
     return kErrorExitCode;
@@ -783,9 +785,9 @@ static int CreateIsolateAndSnapshot(const CommandLineOptions& inputs) {
   // If the input dill file does not have a root library, then
   // Dart_LoadScript will error.
   //
-  // TODO(kernel): Dart_CreateIsolateFromKernel should respect the root library
-  // in the kernel file, though this requires auditing the other loading paths
-  // in the embedders that had to work around this.
+  // TODO(kernel): Dart_CreateIsolateGroupFromKernel should respect the root
+  // library in the kernel file, though this requires auditing the other
+  // loading paths in the embedders that had to work around this.
   result = Dart_SetRootLibrary(
       Dart_LoadLibraryFromKernel(kernel_buffer, kernel_buffer_size));
   CHECK_RESULT(result);

runtime/bin/isolate_data.cc

@@ -9,45 +9,54 @@
 namespace dart {
 namespace bin {
 
-IsolateData::IsolateData(const char* url,
-                         const char* package_root,
-                         const char* packages_file,
-                         AppSnapshot* app_snapshot)
+IsolateGroupData::IsolateGroupData(const char* url,
+                                   const char* package_root,
+                                   const char* packages_file,
+                                   AppSnapshot* app_snapshot,
+                                   bool isolate_run_app_snapshot)
     : script_url((url != NULL) ? strdup(url) : NULL),
       package_root(NULL),
-      packages_file(NULL),
-      loader_(NULL),
       app_snapshot_(app_snapshot),
       dependencies_(NULL),
       resolved_packages_config_(NULL),
       kernel_buffer_(NULL),
-      kernel_buffer_size_(0) {
+      kernel_buffer_size_(0),
+      isolate_run_app_snapshot_(isolate_run_app_snapshot) {
   if (package_root != NULL) {
     ASSERT(packages_file == NULL);
-    this->package_root = strdup(package_root);
+    package_root = strdup(package_root);
   } else if (packages_file != NULL) {
-    this->packages_file = strdup(packages_file);
+    packages_file_ = strdup(packages_file);
   }
 }
 
-void IsolateData::OnIsolateShutdown() {
-}
-
-IsolateData::~IsolateData() {
+IsolateGroupData::~IsolateGroupData() {
   free(script_url);
   script_url = NULL;
   free(package_root);
   package_root = NULL;
-  free(packages_file);
-  packages_file = NULL;
+  free(packages_file_);
+  packages_file_ = NULL;
   free(resolved_packages_config_);
   resolved_packages_config_ = NULL;
   kernel_buffer_ = NULL;
   kernel_buffer_size_ = 0;
-  delete app_snapshot_;
-  app_snapshot_ = NULL;
   delete dependencies_;
 }
 
+IsolateData::IsolateData(IsolateGroupData* isolate_group_data)
+    : isolate_group_data_(isolate_group_data),
+      loader_(nullptr),
+      packages_file_(nullptr) {
+  if (isolate_group_data->packages_file_ != nullptr) {
+    packages_file_ = strdup(isolate_group_data->packages_file_);
+  }
+}
+
+IsolateData::~IsolateData() {
+  free(packages_file_);
+  packages_file_ = nullptr;
+}
+
 }  // namespace bin
 }  // namespace dart

runtime/bin/isolate_data.h

@@ -28,62 +28,54 @@ class AppSnapshot;
 class EventHandler;
 class Loader;
 
-// Data associated with every isolate in the standalone VM
+// Data associated with every isolate group in the standalone VM
 // embedding. This is used to free external resources for each isolate
-// when the isolate shuts down.
-class IsolateData {
+// group when the isolate group shuts down.
+class IsolateGroupData {
  public:
-  IsolateData(const char* url,
-              const char* package_root,
-              const char* packages_file,
-              AppSnapshot* app_snapshot);
-  ~IsolateData();
+  IsolateGroupData(const char* url,
+                   const char* package_root,
+                   const char* packages_file,
+                   AppSnapshot* app_snapshot,
+                   bool isolate_run_app_snapshot);
+  ~IsolateGroupData();
 
   char* script_url;
   char* package_root;
-  char* packages_file;
 
   const std::shared_ptr<uint8_t>& kernel_buffer() const {
     return kernel_buffer_;
   }
 
   intptr_t kernel_buffer_size() const { return kernel_buffer_size_; }
 
-  // Associate the given kernel buffer with this IsolateData without giving it
-  // ownership of the buffer.
+  // Associate the given kernel buffer with this IsolateGroupData without
+  // giving it ownership of the buffer.
   void SetKernelBufferUnowned(uint8_t* buffer, intptr_t size) {
     ASSERT(kernel_buffer_.get() == NULL);
     kernel_buffer_ = std::shared_ptr<uint8_t>(buffer, FreeUnownedKernelBuffer);
     kernel_buffer_size_ = size;
   }
 
-  // Associate the given kernel buffer with this IsolateData and give it
-  // ownership of the buffer. This IsolateData is the first one to own the
+  // Associate the given kernel buffer with this IsolateGroupData and give it
+  // ownership of the buffer. This IsolateGroupData is the first one to own the
   // buffer.
   void SetKernelBufferNewlyOwned(uint8_t* buffer, intptr_t size) {
     ASSERT(kernel_buffer_.get() == NULL);
     kernel_buffer_ = std::shared_ptr<uint8_t>(buffer, free);
     kernel_buffer_size_ = size;
   }
 
-  // Associate the given kernel buffer with this IsolateData and give it
+  // Associate the given kernel buffer with this IsolateGroupData and give it
   // ownership of the buffer. The buffer is already owned by another
-  // IsolateData.
+  // IsolateGroupData.
   void SetKernelBufferAlreadyOwned(std::shared_ptr<uint8_t> buffer,
                                    intptr_t size) {
     ASSERT(kernel_buffer_.get() == NULL);
     kernel_buffer_ = std::move(buffer);
     kernel_buffer_size_ = size;
   }
 
-  void UpdatePackagesFile(const char* packages_file_) {
-    if (packages_file != NULL) {
-      free(packages_file);
-      packages_file = NULL;
-    }
-    packages_file = strdup(packages_file_);
-  }
-
   const char* resolved_packages_config() const {
     return resolved_packages_config_;
   }
@@ -96,6 +88,54 @@ class IsolateData {
     resolved_packages_config_ = strdup(packages_config);
   }
 
+  MallocGrowableArray<char*>* dependencies() const { return dependencies_; }
+  void set_dependencies(MallocGrowableArray<char*>* deps) {
+    dependencies_ = deps;
+  }
+
+  bool RunFromAppSnapshot() const {
+    // If the main isolate is using an app snapshot the [app_snapshot_] pointer
+    // will be still nullptr (see main.cc:CreateIsolateGroupAndSetupHelper)
+    //
+    // Because of thus we have an additional boolean signaling whether the
+    // isolate was started from an app snapshot.
+    return app_snapshot_ != nullptr || isolate_run_app_snapshot_;
+  }
+
+ private:
+  friend class IsolateData;  // For packages_file_
+
+  std::unique_ptr<AppSnapshot> app_snapshot_;
+  MallocGrowableArray<char*>* dependencies_;
+  char* resolved_packages_config_;
+  std::shared_ptr<uint8_t> kernel_buffer_;
+  intptr_t kernel_buffer_size_;
+  char* packages_file_ = nullptr;
+  bool isolate_run_app_snapshot_;
+
+  static void FreeUnownedKernelBuffer(uint8_t*) {}
+
+  DISALLOW_COPY_AND_ASSIGN(IsolateGroupData);
+};
+
+// Data associated with every isolate in the standalone VM
+// embedding. This is used to free external resources for each isolate
+// when the isolate shuts down.
+class IsolateData {
+ public:
+  explicit IsolateData(IsolateGroupData* isolate_group_data);
+  ~IsolateData();
+
+  IsolateGroupData* isolate_group_data() const { return isolate_group_data_; }
+
+  void UpdatePackagesFile(const char* packages_file) {
+    if (packages_file != nullptr) {
+      free(packages_file_);
+      packages_file_ = nullptr;
+    }
+    packages_file_ = strdup(packages_file);
+  }
+
   // While loading a loader is associated with the isolate.
   bool HasLoader() const { return loader_ != NULL; }
   Loader* loader() const {
@@ -106,22 +146,13 @@ class IsolateData {
     ASSERT((loader_ == NULL) || (loader == NULL));
     loader_ = loader;
   }
-  MallocGrowableArray<char*>* dependencies() const { return dependencies_; }
-  void set_dependencies(MallocGrowableArray<char*>* deps) {
-    dependencies_ = deps;
-  }
 
-  void OnIsolateShutdown();
+  const char* packages_file() const { return packages_file_; }
 
  private:
+  IsolateGroupData* isolate_group_data_;
   Loader* loader_;
-  AppSnapshot* app_snapshot_;
-  MallocGrowableArray<char*>* dependencies_;
-  char* resolved_packages_config_;
-  std::shared_ptr<uint8_t> kernel_buffer_;
-  intptr_t kernel_buffer_size_;
-
-  static void FreeUnownedKernelBuffer(uint8_t*) {}
+  char* packages_file_;
 
   DISALLOW_COPY_AND_ASSIGN(IsolateData);
 };

runtime/bin/loader.cc

@@ -263,18 +263,18 @@ static bool PathContainsSeparator(const char* path) {
 
 void Loader::AddDependencyLocked(Loader* loader, const char* resolved_uri) {
   MallocGrowableArray<char*>* dependencies =
-      loader->isolate_data_->dependencies();
+      loader->isolate_group_data()->dependencies();
   if (dependencies == NULL) {
     return;
   }
   dependencies->Add(strdup(resolved_uri));
 }
 
 void Loader::ResolveDependenciesAsFilePaths() {
-  IsolateData* isolate_data =
-      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
-  ASSERT(isolate_data != NULL);
-  MallocGrowableArray<char*>* dependencies = isolate_data->dependencies();
+  IsolateGroupData* isolate_group_data =
+      reinterpret_cast<IsolateGroupData*>(Dart_CurrentIsolateGroupData());
+  ASSERT(isolate_group_data != NULL);
+  MallocGrowableArray<char*>* dependencies = isolate_group_data->dependencies();
   if (dependencies == NULL) {
     return;
   }
@@ -454,15 +454,15 @@ bool Loader::ProcessQueueLocked(ProcessResult process_result) {
   return !hit_error;
 }
 
-void Loader::InitForSnapshot(const char* snapshot_uri) {
-  IsolateData* isolate_data =
-      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+void Loader::InitForSnapshot(const char* snapshot_uri,
+                             IsolateData* isolate_data) {
   ASSERT(isolate_data != NULL);
   ASSERT(!isolate_data->HasLoader());
   // Setup a loader. The constructor does a bunch of leg work.
   Loader* loader = new Loader(isolate_data);
   // Send the init message.
-  loader->Init(isolate_data->package_root, isolate_data->packages_file,
+  loader->Init(isolate_data->isolate_group_data()->package_root,
+               isolate_data->packages_file(),
                DartUtils::original_working_directory, snapshot_uri);
   // Destroy the loader. The destructor does a bunch of leg work.
   delete loader;
@@ -516,15 +516,15 @@ Dart_Handle Loader::SendAndProcessReply(intptr_t tag,
                                         Dart_Handle url,
                                         uint8_t** payload,
                                         intptr_t* payload_length) {
-  IsolateData* isolate_data =
-      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  auto isolate_data = reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
   ASSERT(isolate_data != NULL);
   ASSERT(!isolate_data->HasLoader());
   Loader* loader = NULL;
 
   // Setup the loader. The constructor does a bunch of leg work.
   loader = new Loader(isolate_data);
-  loader->Init(isolate_data->package_root, isolate_data->packages_file,
+  loader->Init(isolate_data->isolate_group_data()->package_root,
+               isolate_data->packages_file(),
                DartUtils::original_working_directory, NULL);
   ASSERT(loader != NULL);
   ASSERT(isolate_data->HasLoader());
@@ -565,6 +565,10 @@ Dart_Handle Loader::ResolveAsFilePath(Dart_Handle url,
                              payload_length);
 }
 
+IsolateGroupData* Loader::isolate_group_data() {
+  return isolate_data_->isolate_group_data();
+}
+
 #if defined(DART_PRECOMPILED_RUNTIME)
 Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
                                       Dart_Handle library,
@@ -679,8 +683,7 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
     }
   }
 
-  IsolateData* isolate_data =
-      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  auto isolate_data = reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
   ASSERT(isolate_data != NULL);
   if ((tag == Dart_kScriptTag) && Dart_IsString(library)) {
     // Update packages file for isolate.
@@ -707,7 +710,8 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
 
     // Setup the loader. The constructor does a bunch of leg work.
     loader = new Loader(isolate_data);
-    loader->Init(isolate_data->package_root, isolate_data->packages_file,
+    loader->Init(isolate_data->isolate_group_data()->package_root,
+                 isolate_data->packages_file(),
                  DartUtils::original_working_directory,
                  (tag == Dart_kScriptTag) ? url_string : NULL);
   } else {