Revert "[vm/concurrency] Introduce concept of Isolate Groups"

This reverts commit b75057b.

Reason for revert: Caused some failures on app-jit builders

Original change's description:
> [vm/concurrency] Introduce concept of Isolate Groups
> 
> 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
> 
> Change-Id: I82437ac017ca33018d45e02f353b0672db155f6a
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105241
> Commit-Queue: Martin Kustermann <kustermann@google.com>
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
> Reviewed-by: Alexander Aprelev <aam@google.com>

TBR=kustermann@google.com,aam@google.com,rmacnak@google.com

Change-Id: Ibd90992a01d61188f27b445c21532e0c46f996ea
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107405
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>

Author: mkustermann

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_CreateIsolateGroupFromKernel(
+  Dart_Isolate kernel_isolate = Dart_CreateIsolateFromKernel(
       data.script_uri, data.main, buffer, buffer_size, data.flags,
-      data.isolate_group_data, data.isolate_data, error);
+      data.callback_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_CreateIsolateGroupFromKernel(
+  Dart_Isolate service_isolate = Dart_CreateIsolateFromKernel(
       data.script_uri, data.main, kernel_buffer, kernel_buffer_size, data.flags,
-      data.isolate_group_data, data.isolate_data, error);
+      data.callback_data, error);
   if (service_isolate == nullptr) {
     return nullptr;
   }

runtime/bin/gen_snapshot.cc

@@ -745,24 +745,23 @@ static int CreateIsolateAndSnapshot(const CommandLineOptions& inputs) {
     isolate_flags.entry_points = no_entry_points;
   }
 
-  auto isolate_group_data = new IsolateGroupData(NULL, NULL, NULL, NULL);
+  IsolateData* isolate_data = new IsolateData(NULL, NULL, NULL, NULL);
   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_CreateIsolateGroupFromKernel(
-        NULL, NULL, kernel_buffer, kernel_buffer_size, &isolate_flags,
-        isolate_group_data, /*isolate_data=*/nullptr, &error);
+    isolate = Dart_CreateIsolateFromKernel(NULL, NULL, kernel_buffer,
+                                           kernel_buffer_size, &isolate_flags,
+                                           isolate_data, &error);
   } else {
-    isolate = Dart_CreateIsolateGroup(NULL, NULL, isolate_snapshot_data,
-                                      isolate_snapshot_instructions, NULL, NULL,
-                                      &isolate_flags, isolate_group_data,
-                                      /*isolate_data=*/nullptr, &error);
+    isolate = Dart_CreateIsolate(NULL, NULL, isolate_snapshot_data,
+                                 isolate_snapshot_instructions, NULL, NULL,
+                                 &isolate_flags, isolate_data, &error);
   }
   if (isolate == NULL) {
-    delete isolate_group_data;
+    delete isolate_data;
     Syslog::PrintErr("%s\n", error);
     free(error);
     return kErrorExitCode;
@@ -784,9 +783,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_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.
+  // 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.
   result = Dart_SetRootLibrary(
       Dart_LoadLibraryFromKernel(kernel_buffer, kernel_buffer_size));
   CHECK_RESULT(result);

runtime/bin/isolate_data.cc

@@ -9,10 +9,10 @@
 namespace dart {
 namespace bin {
 
-IsolateGroupData::IsolateGroupData(const char* url,
-                                   const char* package_root,
-                                   const char* packages_file,
-                                   AppSnapshot* app_snapshot)
+IsolateData::IsolateData(const char* url,
+                         const char* package_root,
+                         const char* packages_file,
+                         AppSnapshot* app_snapshot)
     : script_url((url != NULL) ? strdup(url) : NULL),
       package_root(NULL),
       packages_file(NULL),
@@ -30,7 +30,10 @@ IsolateGroupData::IsolateGroupData(const char* url,
   }
 }
 
-IsolateGroupData::~IsolateGroupData() {
+void IsolateData::OnIsolateShutdown() {
+}
+
+IsolateData::~IsolateData() {
   free(script_url);
   script_url = NULL;
   free(package_root);

runtime/bin/isolate_data.h

@@ -31,13 +31,13 @@ class Loader;
 // 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 IsolateGroupData {
+class IsolateData {
  public:
-  IsolateGroupData(const char* url,
-                   const char* package_root,
-                   const char* packages_file,
-                   AppSnapshot* app_snapshot);
-  ~IsolateGroupData();
+  IsolateData(const char* url,
+              const char* package_root,
+              const char* packages_file,
+              AppSnapshot* app_snapshot);
+  ~IsolateData();
 
   char* script_url;
   char* package_root;
@@ -49,26 +49,26 @@ class IsolateGroupData {
 
   intptr_t kernel_buffer_size() const { return kernel_buffer_size_; }
 
-  // Associate the given kernel buffer with this IsolateGroupData without
-  // giving it ownership of the buffer.
+  // Associate the given kernel buffer with this IsolateData 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 IsolateGroupData and give it
-  // ownership of the buffer. This IsolateGroupData is the first one to own the
+  // Associate the given kernel buffer with this IsolateData and give it
+  // ownership of the buffer. This IsolateData 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 IsolateGroupData and give it
+  // Associate the given kernel buffer with this IsolateData and give it
   // ownership of the buffer. The buffer is already owned by another
-  // IsolateGroupData.
+  // IsolateData.
   void SetKernelBufferAlreadyOwned(std::shared_ptr<uint8_t> buffer,
                                    intptr_t size) {
     ASSERT(kernel_buffer_.get() == NULL);
@@ -111,7 +111,7 @@ class IsolateGroupData {
     dependencies_ = deps;
   }
 
-  bool RunFromAppSnapshot() const { return app_snapshot_ != nullptr; }
+  void OnIsolateShutdown();
 
  private:
   Loader* loader_;
@@ -123,7 +123,7 @@ class IsolateGroupData {
 
   static void FreeUnownedKernelBuffer(uint8_t*) {}
 
-  DISALLOW_COPY_AND_ASSIGN(IsolateGroupData);
+  DISALLOW_COPY_AND_ASSIGN(IsolateData);
 };
 
 }  // namespace bin

runtime/bin/loader.cc

@@ -29,9 +29,9 @@ extern DFE dfe;
 static const intptr_t _Dart_kImportExtension = 9;
 static const intptr_t _Dart_kResolveAsFilePath = 10;
 
-Loader::Loader(IsolateGroupData* isolate_group_data)
+Loader::Loader(IsolateData* isolate_data)
     : port_(ILLEGAL_PORT),
-      isolate_group_data_(isolate_group_data),
+      isolate_data_(isolate_data),
       error_(Dart_Null()),
       monitor_(),
       pending_operations_(0),
@@ -40,10 +40,10 @@ Loader::Loader(IsolateGroupData* isolate_group_data)
       results_capacity_(0),
       payload_(NULL),
       payload_length_(0) {
-  ASSERT(isolate_group_data_ != NULL);
+  ASSERT(isolate_data_ != NULL);
   port_ = Dart_NewNativePort("Loader", Loader::NativeMessageHandler, false);
-  isolate_group_data_->set_loader(this);
-  AddLoader(port_, isolate_group_data_);
+  isolate_data_->set_loader(this);
+  AddLoader(port_, isolate_data_);
 }
 
 Loader::~Loader() {
@@ -55,8 +55,8 @@ Loader::~Loader() {
   monitor_.Exit();
   RemoveLoader(port_);
   port_ = ILLEGAL_PORT;
-  isolate_group_data_->set_loader(NULL);
-  isolate_group_data_ = NULL;
+  isolate_data_->set_loader(NULL);
+  isolate_data_ = NULL;
   for (intptr_t i = 0; i < results_length_; i++) {
     results_[i].Cleanup();
   }
@@ -263,18 +263,18 @@ static bool PathContainsSeparator(const char* path) {
 
 void Loader::AddDependencyLocked(Loader* loader, const char* resolved_uri) {
   MallocGrowableArray<char*>* dependencies =
-      loader->isolate_group_data_->dependencies();
+      loader->isolate_data_->dependencies();
   if (dependencies == NULL) {
     return;
   }
   dependencies->Add(strdup(resolved_uri));
 }
 
 void Loader::ResolveDependenciesAsFilePaths() {
-  IsolateGroupData* isolate_group_data =
-      reinterpret_cast<IsolateGroupData*>(Dart_CurrentIsolateGroupData());
-  ASSERT(isolate_group_data != NULL);
-  MallocGrowableArray<char*>* dependencies = isolate_group_data->dependencies();
+  IsolateData* isolate_data =
+      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  ASSERT(isolate_data != NULL);
+  MallocGrowableArray<char*>* dependencies = isolate_data->dependencies();
   if (dependencies == NULL) {
     return;
   }
@@ -455,15 +455,14 @@ bool Loader::ProcessQueueLocked(ProcessResult process_result) {
 }
 
 void Loader::InitForSnapshot(const char* snapshot_uri) {
-  IsolateGroupData* isolate_group_data =
-      reinterpret_cast<IsolateGroupData*>(Dart_CurrentIsolateGroupData());
-  ASSERT(isolate_group_data != NULL);
-  ASSERT(!isolate_group_data->HasLoader());
+  IsolateData* isolate_data =
+      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  ASSERT(isolate_data != NULL);
+  ASSERT(!isolate_data->HasLoader());
   // Setup a loader. The constructor does a bunch of leg work.
-  Loader* loader = new Loader(isolate_group_data);
+  Loader* loader = new Loader(isolate_data);
   // Send the init message.
-  loader->Init(isolate_group_data->package_root,
-               isolate_group_data->packages_file,
+  loader->Init(isolate_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;
@@ -517,19 +516,18 @@ Dart_Handle Loader::SendAndProcessReply(intptr_t tag,
                                         Dart_Handle url,
                                         uint8_t** payload,
                                         intptr_t* payload_length) {
-  IsolateGroupData* isolate_group_data =
-      reinterpret_cast<IsolateGroupData*>(Dart_CurrentIsolateGroupData());
-  ASSERT(isolate_group_data != NULL);
-  ASSERT(!isolate_group_data->HasLoader());
+  IsolateData* 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_group_data);
-  loader->Init(isolate_group_data->package_root,
-               isolate_group_data->packages_file,
+  loader = new Loader(isolate_data);
+  loader->Init(isolate_data->package_root, isolate_data->packages_file,
                DartUtils::original_working_directory, NULL);
   ASSERT(loader != NULL);
-  ASSERT(isolate_group_data->HasLoader());
+  ASSERT(isolate_data->HasLoader());
 
   // Now send a load request to the service isolate.
   loader->SendRequest(tag, url, Dart_Null());
@@ -681,47 +679,46 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
     }
   }
 
-  IsolateGroupData* isolate_group_data =
-      reinterpret_cast<IsolateGroupData*>(Dart_CurrentIsolateGroupData());
-  ASSERT(isolate_group_data != NULL);
+  IsolateData* isolate_data =
+      reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
+  ASSERT(isolate_data != NULL);
   if ((tag == Dart_kScriptTag) && Dart_IsString(library)) {
     // Update packages file for isolate.
     const char* packages_file = NULL;
     Dart_Handle result = Dart_StringToCString(library, &packages_file);
     if (Dart_IsError(result)) {
       return result;
     }
-    isolate_group_data->UpdatePackagesFile(packages_file);
+    isolate_data->UpdatePackagesFile(packages_file);
   }
   // Grab this isolate's loader.
   Loader* loader = NULL;
 
   // The outer invocation of the tag handler for this isolate. We make the outer
   // invocation block and allow any nested invocations to operate in parallel.
-  const bool blocking_call = !isolate_group_data->HasLoader();
+  const bool blocking_call = !isolate_data->HasLoader();
 
   // If we are the outer invocation of the tag handler and the tag is an import
   // this means that we are starting a deferred library load.
   const bool is_deferred_import = blocking_call && (tag == Dart_kImportTag);
-  if (!isolate_group_data->HasLoader()) {
+  if (!isolate_data->HasLoader()) {
     // The isolate doesn't have a loader -- this is the outer invocation which
     // will block.
 
     // Setup the loader. The constructor does a bunch of leg work.
-    loader = new Loader(isolate_group_data);
-    loader->Init(isolate_group_data->package_root,
-                 isolate_group_data->packages_file,
+    loader = new Loader(isolate_data);
+    loader->Init(isolate_data->package_root, isolate_data->packages_file,
                  DartUtils::original_working_directory,
                  (tag == Dart_kScriptTag) ? url_string : NULL);
   } else {
     ASSERT(tag != Dart_kScriptTag);
     // The isolate has a loader -- this is an inner invocation that will queue
     // work with the service isolate.
     // Use the existing loader.
-    loader = isolate_group_data->loader();
+    loader = isolate_data->loader();
   }
   ASSERT(loader != NULL);
-  ASSERT(isolate_group_data->HasLoader());
+  ASSERT(isolate_data->HasLoader());
 
   if (DartUtils::IsDartExtensionSchemeURL(url_string)) {
     loader->SendImportExtensionRequest(url, Dart_LibraryUrl(library));
@@ -818,11 +815,11 @@ Loader::LoaderInfo* Loader::loader_infos_ = NULL;
 intptr_t Loader::loader_infos_length_ = 0;
 intptr_t Loader::loader_infos_capacity_ = 0;
 
-// Add a mapping from |port| to |isolate_group_data| (really the loader). When a
+// Add a mapping from |port| to |isolate_data| (really the loader). When a
 // native message arrives, we use this map to report the I/O result to the
 // correct loader.
 // This happens whenever an isolate begins loading.
-void Loader::AddLoader(Dart_Port port, IsolateGroupData* isolate_group_data) {
+void Loader::AddLoader(Dart_Port port, IsolateData* isolate_data) {
   MutexLocker ml(loader_infos_lock_);
   ASSERT(LoaderForLocked(port) == NULL);
   if (loader_infos_length_ == loader_infos_capacity_) {
@@ -835,12 +832,12 @@ void Loader::AddLoader(Dart_Port port, IsolateGroupData* isolate_group_data) {
     // Initialize new entries.
     for (intptr_t i = loader_infos_length_; i < loader_infos_capacity_; i++) {
       loader_infos_[i].port = ILLEGAL_PORT;
-      loader_infos_[i].isolate_group_data = NULL;
+      loader_infos_[i].isolate_data = NULL;
     }
   }
   ASSERT(loader_infos_length_ < loader_infos_capacity_);
   loader_infos_[loader_infos_length_].port = port;
-  loader_infos_[loader_infos_length_].isolate_group_data = isolate_group_data;
+  loader_infos_[loader_infos_length_].isolate_data = isolate_data;
   loader_infos_length_++;
   ASSERT(LoaderForLocked(port) != NULL);
 }
@@ -874,7 +871,7 @@ Loader* Loader::LoaderForLocked(Dart_Port port) {
   if (index < 0) {
     return NULL;
   }
-  return loader_infos_[index].isolate_group_data->loader();
+  return loader_infos_[index].isolate_data->loader();
 }
 
 Loader* Loader::LoaderFor(Dart_Port port) {