VM: Fix an app-jit related shutdown race.

IsolateData contains AppSnapshot which might own some HeapPage-s, so we can't
destroy IsolateData in the Dart_IsolateShutdownCallback, because some thread
running in the isolate (e.g. background compiler) might still touch
thoses pages or objects they host.

We need to delay destruction of AppSnapshot until after the isolate shutdown.

Current API does not allow that, so we introduce a new isolate lifecycle
callback - Dart_IsolateCleanupCallback, which is invoked at the end of the
isolote lifecycle when things like background compiler have been stopped
and no Dart code is supposed to run.

BUG=
R=rmacnak@google.com

Review-Url: https://codereview.chromium.org/2720723005 .

runtime/bin/isolate_data.cc

@@ -29,6 +29,14 @@ IsolateData::IsolateData(const char* url,
 }
 
 
+void IsolateData::OnIsolateShutdown() {
+  if (builtin_lib_ != NULL) {
+    Dart_DeletePersistentHandle(builtin_lib_);
+    builtin_lib_ = NULL;
+  }
+}
+
+
 IsolateData::~IsolateData() {
   free(script_url);
   script_url = NULL;
@@ -38,9 +46,6 @@ IsolateData::~IsolateData() {
   packages_file = NULL;
   free(udp_receive_buffer);
   udp_receive_buffer = NULL;
-  if (builtin_lib_ != NULL) {
-    Dart_DeletePersistentHandle(builtin_lib_);
-  }
   delete app_snapshot_;
   app_snapshot_ = NULL;
 }

runtime/bin/isolate_data.h

@@ -68,6 +68,8 @@ class IsolateData {
     dependencies_ = deps;
   }
 
+  void OnIsolateShutdown();
+
  private:
   Dart_Handle builtin_lib_;
   Loader* loader_;

runtime/bin/main.cc

@@ -1150,7 +1150,14 @@ char* BuildIsolateName(const char* script_name, const char* func_name) {
   return buffer;
 }
 
-static void ShutdownIsolate(void* callback_data) {
+
+static void OnIsolateShutdown(void* callback_data) {
+  IsolateData* isolate_data = reinterpret_cast<IsolateData*>(callback_data);
+  isolate_data->OnIsolateShutdown();
+}
+
+
+static void DeleteIsolateData(void* callback_data) {
   IsolateData* isolate_data = reinterpret_cast<IsolateData*>(callback_data);
   delete isolate_data;
 }
@@ -1613,7 +1620,8 @@ void main(int argc, char** argv) {
   init_params.vm_snapshot_data = vm_snapshot_data;
   init_params.vm_snapshot_instructions = vm_snapshot_instructions;
   init_params.create = CreateIsolateAndSetup;
-  init_params.shutdown = ShutdownIsolate;
+  init_params.shutdown = OnIsolateShutdown;
+  init_params.cleanup = DeleteIsolateData;
   init_params.file_open = DartUtils::OpenFile;
   init_params.file_read = DartUtils::ReadFile;
   init_params.file_write = DartUtils::WriteFile;

runtime/bin/run_vm_tests.cc

@@ -123,7 +123,7 @@ static int Main(int argc, const char** argv) {
   ASSERT(set_vm_flags_success);
   const char* err_msg = Dart::InitOnce(
       dart::bin::vm_snapshot_data, dart::bin::vm_snapshot_instructions, NULL,
-      NULL, NULL, dart::bin::DartUtils::OpenFile,
+      NULL, NULL, NULL, dart::bin::DartUtils::OpenFile,
       dart::bin::DartUtils::ReadFile, dart::bin::DartUtils::WriteFile,
       dart::bin::DartUtils::CloseFile, NULL, NULL);
   ASSERT(err_msg == NULL);

runtime/include/dart_api.h

@@ -707,6 +707,22 @@ typedef void (*Dart_IsolateUnhandledExceptionCallback)(Dart_Handle error);
  */
 typedef void (*Dart_IsolateShutdownCallback)(void* callback_data);
 
+/**
+ * An isolate cleanup callback function.
+ *
+ * This callback, provided by the embedder, is called after the vm
+ * shuts down an isolate. There will be no current isolate and it is *not*
+ * safe to run Dart code.
+ *
+ * This function should be used to dispose of native resources that
+ * are allocated to an isolate in order to avoid leaks.
+ *
+ * \param callback_data The same callback data which was passed to the
+ *   isolate when it was created.
+ *
+ */
+typedef void (*Dart_IsolateCleanupCallback)(void* callback_data);
+
 /**
  * A thread death callback function.
  * This callback, provided by the embedder, is called before a thread in the
@@ -788,6 +804,8 @@ typedef Dart_Handle (*Dart_GetVMServiceAssetsArchive)();
  *   See Dart_IsolateCreateCallback.
  * \param shutdown A function to be called when an isolate is shutdown.
  *   See Dart_IsolateShutdownCallback.
+ * \param cleanup A function to be called after an isolate is shutdown.
+ *   See Dart_IsolateCleanupCallback.
  * \param get_service_assets A function to be called by the service isolate when
  *    it requires the vmservice assets archive.
  *    See Dart_GetVMServiceAssetsArchive.
@@ -798,6 +816,7 @@ typedef struct {
   const uint8_t* vm_snapshot_instructions;
   Dart_IsolateCreateCallback create;
   Dart_IsolateShutdownCallback shutdown;
+  Dart_IsolateCleanupCallback cleanup;
   Dart_ThreadExitCallback thread_exit;
   Dart_FileOpenCallback file_open;
   Dart_FileReadCallback file_read;

runtime/vm/dart.cc

@@ -125,6 +125,7 @@ char* Dart::InitOnce(const uint8_t* vm_isolate_snapshot,
                      const uint8_t* instructions_snapshot,
                      Dart_IsolateCreateCallback create,
                      Dart_IsolateShutdownCallback shutdown,
+                     Dart_IsolateCleanupCallback cleanup,
                      Dart_ThreadExitCallback thread_exit,
                      Dart_FileOpenCallback file_open,
                      Dart_FileReadCallback file_read,
@@ -313,6 +314,7 @@ char* Dart::InitOnce(const uint8_t* vm_isolate_snapshot,
   Thread::ExitIsolate();  // Unregister the VM isolate from this thread.
   Isolate::SetCreateCallback(create);
   Isolate::SetShutdownCallback(shutdown);
+  Isolate::SetCleanupCallback(cleanup);
 
   if (FLAG_support_service) {
     Service::SetGetServiceAssetsCallback(get_service_assets);

runtime/vm/dart.h

@@ -28,6 +28,7 @@ class Dart : public AllStatic {
                         const uint8_t* vm_snapshot_instructions,
                         Dart_IsolateCreateCallback create,
                         Dart_IsolateShutdownCallback shutdown,
+                        Dart_IsolateCleanupCallback cleanup,
                         Dart_ThreadExitCallback thread_exit,
                         Dart_FileOpenCallback file_open,
                         Dart_FileReadCallback file_read,

runtime/vm/dart_api_impl.cc

@@ -1192,9 +1192,9 @@ DART_EXPORT char* Dart_Initialize(Dart_InitializeParams* params) {
 
   return Dart::InitOnce(
       params->vm_snapshot_data, params->vm_snapshot_instructions,
-      params->create, params->shutdown, params->thread_exit, params->file_open,
-      params->file_read, params->file_write, params->file_close,
-      params->entropy_source, params->get_service_assets);
+      params->create, params->shutdown, params->cleanup, params->thread_exit,
+      params->file_open, params->file_read, params->file_write,
+      params->file_close, params->entropy_source, params->get_service_assets);
 }
 
 

runtime/vm/isolate.cc

@@ -1780,11 +1780,17 @@ void Isolate::Shutdown() {
   // TODO(5411455): For now just make sure there are no current isolates
   // as we are shutting down the isolate.
   Thread::ExitIsolate();
+
+  Dart_IsolateCleanupCallback cleanup = Isolate::CleanupCallback();
+  if (cleanup != NULL) {
+    cleanup(init_callback_data());
+  }
 }
 
 
 Dart_IsolateCreateCallback Isolate::create_callback_ = NULL;
 Dart_IsolateShutdownCallback Isolate::shutdown_callback_ = NULL;
+Dart_IsolateCleanupCallback Isolate::cleanup_callback_ = NULL;
 
 Monitor* Isolate::isolates_list_monitor_ = NULL;
 Isolate* Isolate::isolates_list_head_ = NULL;

runtime/vm/isolate.h

@@ -399,6 +399,14 @@ class Isolate : public BaseIsolate {
     return shutdown_callback_;
   }
 
+  static void SetCleanupCallback(Dart_IsolateCleanupCallback cb) {
+    cleanup_callback_ = cb;
+  }
+  static Dart_IsolateCleanupCallback CleanupCallback() {
+    return cleanup_callback_;
+  }
+
+
   void set_object_id_ring(ObjectIdRing* ring) { object_id_ring_ = ring; }
   ObjectIdRing* object_id_ring() { return object_id_ring_; }
 
@@ -851,6 +859,7 @@ class Isolate : public BaseIsolate {
 
   static Dart_IsolateCreateCallback create_callback_;
   static Dart_IsolateShutdownCallback shutdown_callback_;
+  static Dart_IsolateCleanupCallback cleanup_callback_;
 
   static void WakePauseEventHandler(Dart_Isolate isolate);