Revert "Load isolate from parent's kernel in Isolate.spawn calls."

This reverts commit e143a52.

Reason for revert: Broke all the reload-kernel bots.

Original change's description:
> Load isolate from parent's kernel in Isolate.spawn calls.
> 
> Bug: #6610
> Change-Id: Icd6e1c5d6d4b64b611fc58333c051a8a9a50679d
> Reviewed-on: https://dart-review.googlesource.com/c/85724
> Commit-Queue: Liam Appelbe <[email protected]>
> Reviewed-by: Vyacheslav Egorov <[email protected]>
> Reviewed-by: Ryan Macnak <[email protected]>

[email protected],[email protected],[email protected],[email protected]

Change-Id: Ie9c305256da9b6478153b99502d0c63b0f43a3e6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: #6610
Reviewed-on: https://dart-review.googlesource.com/c/86082
Reviewed-by: Liam Appelbe <[email protected]>
Commit-Queue: Liam Appelbe <[email protected]>

Author: liamappelbe

runtime/bin/isolate_data.cc

@@ -21,7 +21,8 @@ IsolateData::IsolateData(const char* url,
       dependencies_(NULL),
       resolved_packages_config_(NULL),
       kernel_buffer_(NULL),
-      kernel_buffer_size_(0) {
+      kernel_buffer_size_(0),
+      owns_kernel_buffer_(false) {
   if (package_root != NULL) {
     ASSERT(packages_file == NULL);
     this->package_root = strdup(package_root);
@@ -42,6 +43,10 @@ IsolateData::~IsolateData() {
   packages_file = NULL;
   free(resolved_packages_config_);
   resolved_packages_config_ = NULL;
+  if (owns_kernel_buffer_) {
+    ASSERT(kernel_buffer_ != NULL);
+    free(kernel_buffer_);
+  }
   kernel_buffer_ = NULL;
   kernel_buffer_size_ = 0;
   delete app_snapshot_;

runtime/bin/isolate_data.h

@@ -5,9 +5,6 @@
 #ifndef RUNTIME_BIN_ISOLATE_DATA_H_
 #define RUNTIME_BIN_ISOLATE_DATA_H_
 
-#include <memory>
-#include <utility>
-
 #include "include/dart_api.h"
 #include "platform/assert.h"
 #include "platform/globals.h"
@@ -43,37 +40,13 @@ class IsolateData {
   char* package_root;
   char* packages_file;
 
-  const std::shared_ptr<uint8_t>& kernel_buffer() const {
-    return kernel_buffer_;
-  }
-
+  const 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.
-  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
-  // 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
-  // ownership of the buffer. The buffer is already owned by another
-  // IsolateData.
-  void SetKernelBufferAlreadyOwned(std::shared_ptr<uint8_t> buffer,
-                                   intptr_t size) {
-    ASSERT(kernel_buffer_.get() == NULL);
-    kernel_buffer_ = std::move(buffer);
+  void set_kernel_buffer(uint8_t* buffer, intptr_t size, bool take_ownership) {
+    ASSERT(kernel_buffer_ == NULL);
+    kernel_buffer_ = buffer;
     kernel_buffer_size_ = size;
+    owns_kernel_buffer_ = take_ownership;
   }
 
   void UpdatePackagesFile(const char* packages_file_) {
@@ -118,10 +91,9 @@ class IsolateData {
   AppSnapshot* app_snapshot_;
   MallocGrowableArray<char*>* dependencies_;
   char* resolved_packages_config_;
-  std::shared_ptr<uint8_t> kernel_buffer_;
+  uint8_t* kernel_buffer_;
   intptr_t kernel_buffer_size_;
-
-  static void FreeUnownedKernelBuffer(uint8_t*) {}
+  bool owns_kernel_buffer_;
 
   DISALLOW_COPY_AND_ASSIGN(IsolateData);
 };

runtime/bin/main.cc

@@ -5,7 +5,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <memory>
 
 #include "include/bin/dart_io_api.h"
 #include "include/dart_api.h"
@@ -214,7 +213,7 @@ static Dart_Isolate IsolateSetupHelper(Dart_Isolate isolate,
 #if !defined(DART_PRECOMPILED_RUNTIME)
   IsolateData* isolate_data =
       reinterpret_cast<IsolateData*>(Dart_IsolateData(isolate));
-  const uint8_t* kernel_buffer = isolate_data->kernel_buffer().get();
+  const uint8_t* kernel_buffer = isolate_data->kernel_buffer();
   intptr_t kernel_buffer_size = isolate_data->kernel_buffer_size();
 #endif
 
@@ -275,8 +274,9 @@ static Dart_Isolate IsolateSetupHelper(Dart_Isolate isolate,
       Dart_ShutdownIsolate();
       return NULL;
     }
-    isolate_data->SetKernelBufferNewlyOwned(application_kernel_buffer,
-                                            application_kernel_buffer_size);
+    isolate_data->set_kernel_buffer(application_kernel_buffer,
+                                    application_kernel_buffer_size,
+                                    true /*take ownership*/);
     kernel_buffer = application_kernel_buffer;
     kernel_buffer_size = application_kernel_buffer_size;
   }
@@ -433,9 +433,9 @@ static Dart_Isolate CreateAndSetupKernelIsolate(const char* script_uri,
     dfe.LoadKernelService(&kernel_service_buffer, &kernel_service_buffer_size);
     ASSERT(kernel_service_buffer != NULL);
     isolate_data = new IsolateData(uri, package_root, packages_config, NULL);
-    isolate_data->SetKernelBufferUnowned(
-        const_cast<uint8_t*>(kernel_service_buffer),
-        kernel_service_buffer_size);
+    isolate_data->set_kernel_buffer(const_cast<uint8_t*>(kernel_service_buffer),
+                                    kernel_service_buffer_size,
+                                    false /* take_ownership */);
     isolate = Dart_CreateIsolateFromKernel(
         DART_KERNEL_ISOLATE_NAME, main, kernel_service_buffer,
         kernel_service_buffer_size, flags, isolate_data, error);
@@ -527,13 +527,11 @@ static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
                                                 const char* package_root,
                                                 const char* packages_config,
                                                 Dart_IsolateFlags* flags,
-                                                void* callback_data,
                                                 char** error,
                                                 int* exit_code) {
   int64_t start = Dart_TimelineGetMicros();
   ASSERT(script_uri != NULL);
   uint8_t* kernel_buffer = NULL;
-  std::shared_ptr<uint8_t> parent_kernel_buffer;
   intptr_t kernel_buffer_size = 0;
   AppSnapshot* app_snapshot = NULL;
 
@@ -567,30 +565,16 @@ static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
           &isolate_snapshot_data, &isolate_snapshot_instructions);
     }
   }
-
-  if (flags->copy_parent_code && callback_data) {
-    IsolateData* parent_isolate_data =
-        reinterpret_cast<IsolateData*>(callback_data);
-    parent_kernel_buffer = parent_isolate_data->kernel_buffer();
-    kernel_buffer = parent_kernel_buffer.get();
-    kernel_buffer_size = parent_isolate_data->kernel_buffer_size();
-  }
-
-  if (kernel_buffer == NULL && !isolate_run_app_snapshot) {
+  if (!isolate_run_app_snapshot) {
     dfe.ReadScript(script_uri, &kernel_buffer, &kernel_buffer_size);
   }
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 
   IsolateData* isolate_data =
       new IsolateData(script_uri, package_root, packages_config, app_snapshot);
   if (kernel_buffer != NULL) {
-    if (parent_kernel_buffer) {
-      isolate_data->SetKernelBufferAlreadyOwned(std::move(parent_kernel_buffer),
-                                                kernel_buffer_size);
-    } else {
-      isolate_data->SetKernelBufferNewlyOwned(kernel_buffer,
-                                              kernel_buffer_size);
-    }
+    isolate_data->set_kernel_buffer(kernel_buffer, kernel_buffer_size,
+                                    true /*take ownership*/);
   }
   if (is_main_isolate && (Options::depfile() != NULL)) {
     isolate_data->set_dependencies(new MallocGrowableArray<char*>());
@@ -656,7 +640,7 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
                                           const char* package_root,
                                           const char* package_config,
                                           Dart_IsolateFlags* flags,
-                                          void* callback_data,
+                                          void* data,
                                           char** error) {
   // The VM should never call the isolate helper with a NULL flags.
   ASSERT(flags != NULL);
@@ -683,8 +667,8 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
   }
   bool is_main_isolate = false;
   return CreateIsolateAndSetupHelper(is_main_isolate, script_uri, main,
-                                     package_root, package_config, flags,
-                                     callback_data, error, &exit_code);
+                                     package_root, package_config, flags, error,
+                                     &exit_code);
 }
 
 char* BuildIsolateName(const char* script_name, const char* func_name) {
@@ -818,8 +802,7 @@ bool RunMainIsolate(const char* script_name, CommandLineOptions* dart_options) {
 
   Dart_Isolate isolate = CreateIsolateAndSetupHelper(
       is_main_isolate, script_name, "main", Options::package_root(),
-      Options::packages_file(), &flags, NULL /* callback_data */, &error,
-      &exit_code);
+      Options::packages_file(), &flags, &error, &exit_code);
 
   if (isolate == NULL) {
     delete[] isolate_name;

runtime/bin/run_vm_tests.cc

@@ -166,9 +166,9 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
     ASSERT(kernel_service_buffer != NULL);
     isolate_data =
         new bin::IsolateData(script_uri, package_root, packages_config, NULL);
-    isolate_data->SetKernelBufferUnowned(
-        const_cast<uint8_t*>(kernel_service_buffer),
-        kernel_service_buffer_size);
+    isolate_data->set_kernel_buffer(const_cast<uint8_t*>(kernel_service_buffer),
+                                    kernel_service_buffer_size,
+                                    false /* take_ownership */);
     isolate = Dart_CreateIsolateFromKernel(
         script_uri, main, kernel_service_buffer, kernel_service_buffer_size,
         flags, isolate_data, error);

runtime/include/dart_api.h

@@ -553,7 +553,7 @@ typedef struct {
  * for each part.
  */
 
-#define DART_FLAGS_CURRENT_VERSION (0x0000000b)
+#define DART_FLAGS_CURRENT_VERSION (0x0000000a)
 
 typedef struct {
   int32_t version;
@@ -564,7 +564,6 @@ typedef struct {
   Dart_QualifiedFunctionName* entry_points;
   bool load_vmservice_library;
   bool unsafe_trust_strong_mode_types;
-  bool copy_parent_code;
 } Dart_IsolateFlags;
 
 /**

runtime/lib/isolate.cc

@@ -230,10 +230,6 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnFunction, 10) {
           isolate->spawn_count_monitor(), isolate->spawn_count(),
           utf8_package_root, utf8_package_config, paused.value(), fatal_errors,
           on_exit_port, on_error_port);
-
-      // Since this is a call to Isolate.spawn, copy the parent isolate's code.
-      state->isolate_flags()->copy_parent_code = true;
-
       ThreadPool::Task* spawn_task = new SpawnIsolateTask(state);
 
       isolate->IncrementSpawnCount();
@@ -361,9 +357,6 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnUri, 12) {
     flags->enable_asserts = checked.value();
   }
 
-  // Since this is a call to Isolate.spawnUri, don't copy the parent's code.
-  state->isolate_flags()->copy_parent_code = false;
-
   ThreadPool::Task* spawn_task = new SpawnIsolateTask(state);
 
   isolate->IncrementSpawnCount();

runtime/vm/isolate.cc

@@ -779,7 +779,6 @@ void Isolate::FlagsInitialize(Dart_IsolateFlags* api_flags) {
 #undef INIT_FROM_FLAG
   api_flags->entry_points = NULL;
   api_flags->load_vmservice_library = false;
-  api_flags->copy_parent_code = false;
 }
 
 void Isolate::FlagsCopyTo(Dart_IsolateFlags* api_flags) const {
@@ -790,7 +789,6 @@ void Isolate::FlagsCopyTo(Dart_IsolateFlags* api_flags) const {
 #undef INIT_FROM_FIELD
   api_flags->entry_points = NULL;
   api_flags->load_vmservice_library = should_load_vmservice();
-  api_flags->copy_parent_code = false;
 }
 
 void Isolate::FlagsCopyFrom(const Dart_IsolateFlags& api_flags) {

runtime/vm/isolate.h

@@ -133,7 +133,7 @@ typedef FixedCache<intptr_t, CatchEntryMovesRefPtr, 16> CatchEntryMovesCache;
 // List of Isolate flags with corresponding members of Dart_IsolateFlags and
 // corresponding global command line flags.
 //
-//       V(when, name, bit-name, Dart_IsolateFlags-name, command-line-flag-name)
+//       V(when, name, Dart_IsolateFlags-member-name, command-line-flag-name)
 //
 #define ISOLATE_FLAG_LIST(V)                                                   \
   V(NONPRODUCT, asserts, EnableAsserts, enable_asserts, FLAG_enable_asserts)   \

tests/lib_2/isolate/issue_6610_test.dart

@@ -212,7 +212,6 @@ isolate/isolate_complex_messages_test: Skip # Isolate.spawnUri
 isolate/issue_21398_parent_isolate1_test: Skip # Isolate.spawnUri
 isolate/issue_21398_parent_isolate_test: Skip # Isolate.spawnUri
 isolate/issue_24243_parent_isolate_test: Skip # Isolate.spawnUri
-isolate/issue_6610_test: Skip # Isolate.spawnUri
 isolate/mandel_isolate_test: Skip # Isolate.spawnUri
 isolate/message2_test: Skip # Isolate.spawnUri
 isolate/message_test: Skip # Isolate.spawnUri