Shutdown the service isolate when shutting down the VM

runtime/vm/service_isolate.cc

Index: runtime/vm/service_isolate.cc
diff --git a/runtime/vm/service_isolate.cc b/runtime/vm/service_isolate.cc
index fc9fc4273eb41918f0d42a89b73e8645f91f5859..d681232442850ec971530200be457b61af795cbe 100644
--- a/runtime/vm/service_isolate.cc
+++ b/runtime/vm/service_isolate.cc
@@ -116,6 +116,7 @@ static Dart_Port ExtractPort(Isolate* isolate, Dart_Handle receivePort) {
 
 
 // These must be kept in sync with service/constants.dart
+#define VM_SERVICE_ISOLATE_EXIT_MESSAGE_ID 0
 #define VM_SERVICE_ISOLATE_STARTUP_MESSAGE_ID 1
 #define VM_SERVICE_ISOLATE_SHUTDOWN_MESSAGE_ID 2
 
@@ -134,13 +135,26 @@ static RawArray* MakeServiceControlMessage(Dart_Port port_id, intptr_t code,
 }
 
 
+static RawArray* MakeServiceExitMessage() {
+  const Array& list = Array::Handle(Array::New(1));
+  ASSERT(!list.IsNull());
+  const intptr_t code = VM_SERVICE_ISOLATE_EXIT_MESSAGE_ID;
+  const Integer& code_int = Integer::Handle(Integer::New(code));
+  list.SetAt(0, code_int);
+  return list.raw();
+}
+
+
 const char* ServiceIsolate::kName = "vm-service";
 Isolate* ServiceIsolate::isolate_ = NULL;
 Dart_Port ServiceIsolate::port_ = ILLEGAL_PORT;
 Dart_Port ServiceIsolate::load_port_ = ILLEGAL_PORT;
 Dart_IsolateCreateCallback ServiceIsolate::create_callback_ = NULL;
+uint8_t* ServiceIsolate::exit_message_ = NULL;
+intptr_t ServiceIsolate::exit_message_length_ = 0;
 Monitor* ServiceIsolate::monitor_ = NULL;
 bool ServiceIsolate::initializing_ = true;
+bool ServiceIsolate::shutting_down_ = false;
 
 
 class RegisterRunningIsolatesVisitor : public IsolateVisitor {
@@ -279,6 +293,19 @@ class ServiceIsolateNatives : public AllStatic {
       Isolate::VisitIsolates(&register_isolates);
     }
   }
+
+  static void OnExit(Dart_NativeArguments args) {
+    NativeArguments* arguments = reinterpret_cast<NativeArguments*>(args);
+    Isolate* isolate = arguments->isolate();
+    StackZone zone(isolate);
+    HANDLESCOPE(isolate);
+    {
+      if (FLAG_trace_service) {
+        OS::Print("vm-service: processed exit message.\n");
+      }
+      ServiceIsolate::FinishedExiting();
+    }
+  }
 };
 
 
@@ -298,6 +325,8 @@ static ServiceNativeEntry _ServiceNativeEntries[] = {
     ServiceIsolateNatives::SetEventMask},
   {"VMService_OnStart", 0,
     ServiceIsolateNatives::OnStart },
+  {"VMService_OnExit", 0,
+    ServiceIsolateNatives::OnExit },
 };
 
 
@@ -434,6 +463,23 @@ bool ServiceIsolate::SendIsolateShutdownMessage() {
 }
 
 
+void ServiceIsolate::SendServiceExitMessage() {
+  if (!IsRunning()) {

[zra, 2015/03/24 21:53:59]

Should this be an ASSERT()?

[Cutch, 2015/03/24 23:39:07]

No, we may have failed to startup the service isolate or the service isolate
could have already exited.

+    return;
+  }
+  if ((exit_message_ == NULL) || (exit_message_length_ == 0)) {

[zra, 2015/03/24 21:53:59]

ditto

[Cutch, 2015/03/24 23:39:07]

Similarly.

+    return;
+  }
+  if (FLAG_trace_service) {
+    OS::Print("vm-service: sending service exit message.\n");
+  }
+  PortMap::PostMessage(new Message(port_,
+                                   exit_message_,
+                                   exit_message_length_,
+                                   Message::kNormalPriority));
+}
+
+
 void ServiceIsolate::SetServicePort(Dart_Port port) {
   MonitorLocker ml(monitor_);
   port_ = port;
@@ -506,6 +552,31 @@ void ServiceIsolate::MaybeInjectVMServiceLibrary(Isolate* isolate) {
 }
 
 
+void ServiceIsolate::ConstructExitMessageAndCache(Isolate* isolate) {
+  // Construct and cache exit message here so we can send it without needing an
+  // isolate.
+  StartIsolateScope iso_scope(isolate);
+  StackZone zone(isolate);
+  HANDLESCOPE(isolate);
+  ASSERT(exit_message_ == NULL);
+  ASSERT(exit_message_length_ == 0);
+  const Array& list = Array::Handle(MakeServiceExitMessage());
+  ASSERT(!list.IsNull());
+  MessageWriter writer(&exit_message_, &allocator, false);
+  writer.WriteMessage(list);
+  exit_message_length_ = writer.BytesWritten();
+  ASSERT(exit_message_ != NULL);
+  ASSERT(exit_message_length_ != 0);
+}
+
+
+void ServiceIsolate::FinishedExiting() {
+  MonitorLocker ml(monitor_);
+  shutting_down_ = false;
+  ml.NotifyAll();
+}
+
+
 void ServiceIsolate::FinishedInitializing() {
   MonitorLocker ml(monitor_);
   initializing_ = false;
@@ -543,6 +614,8 @@ class RunServiceTask : public ThreadPool::Task {
 
     Isolate::SetCurrent(NULL);
 
+    ServiceIsolate::ConstructExitMessageAndCache(isolate);
+
     RunMain(isolate);
 
     ServiceIsolate::FinishedInitializing();
@@ -642,6 +715,24 @@ void ServiceIsolate::Run() {
 }
 
 
+void ServiceIsolate::Shutdown() {
+  if (!IsRunning()) {

[zra, 2015/03/24 21:53:59]

ditto

[Cutch, 2015/03/24 23:39:07]

ditto.

+    return;
+  }
+  {
+    MonitorLocker ml(monitor_);
+    shutting_down_ = true;
+  }
+  SendServiceExitMessage();
+  {
+    MonitorLocker ml(monitor_);
+    while (shutting_down_ && (port_ != ILLEGAL_PORT)) {

[zra, 2015/03/24 21:53:59]

Why would port_ be illegal?

[Cutch, 2015/03/24 23:39:07]

Before the service isolate starts up and after it shuts down the port_ is
ILLEGAL_PORT (the canonical value for an uninitialized Dart_Port).

+      ml.Wait();
+    }
+  }
+}
+
+
 Dart_Handle ServiceIsolate::GetSource(const char* name) {
   ASSERT(name != NULL);
   int i = 0;