[vm] Improve error message from Dart_InvokeVMServiceMethod()

Change-Id: Ib7dad8a4e7fcc7ebb224bee848728e235f04459e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114508
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Clement Skau <cskau@google.com>

runtime/vm/native_api_impl.cc

@@ -146,7 +146,8 @@ DART_EXPORT bool Dart_InvokeVMServiceMethod(uint8_t* request_json,
   // will later on notify once the answer has been received.
   MonitorLocker monitor(vm_service_call_monitor);
 
-  if (ServiceIsolate::SendServiceRpc(request_json, request_json_length, port)) {
+  if (ServiceIsolate::SendServiceRpc(request_json, request_json_length, port,
+                                     error)) {
     // We posted successfully and expect the vm-service to send the reply, so
     // we will wait for it now.
     auto wait_result = monitor.Wait();
@@ -170,14 +171,6 @@ DART_EXPORT bool Dart_InvokeVMServiceMethod(uint8_t* request_json,
     // We couldn't post the message and will not receive any reply. Therefore we
     // clean up the port and return an error.
     Dart_CloseNativePort(port);
-
-    if (error != nullptr) {
-      if (ServiceIsolate::Port() == ILLEGAL_PORT) {
-        *error = strdup("No service isolate port was found.");
-      } else {
-        *error = strdup("Was unable to post message to service isolate.");
-      }
-    }
     return false;
   }
 }

runtime/vm/service_isolate.cc

@@ -81,6 +81,7 @@ Dart_Port ServiceIsolate::port_ = ILLEGAL_PORT;
 Dart_Port ServiceIsolate::load_port_ = ILLEGAL_PORT;
 Dart_Port ServiceIsolate::origin_ = ILLEGAL_PORT;
 char* ServiceIsolate::server_address_ = NULL;
+char* ServiceIsolate::startup_failure_reason_ = nullptr;
 
 void ServiceIsolate::RequestServerInfo(const SendPort& sp) {
   const Array& message = Array::Handle(MakeServerControlMessage(
@@ -128,7 +129,7 @@ bool ServiceIsolate::IsRunning() {
 
 bool ServiceIsolate::IsServiceIsolate(const Isolate* isolate) {
   MonitorLocker ml(monitor_);
-  return isolate == isolate_;
+  return isolate != nullptr && isolate == isolate_;
 }
 
 bool ServiceIsolate::IsServiceIsolateDescendant(const Isolate* isolate) {
@@ -161,7 +162,8 @@ Dart_Port ServiceIsolate::LoadPort() {
 
 bool ServiceIsolate::SendServiceRpc(uint8_t* request_json,
                                     intptr_t request_json_length,
-                                    Dart_Port reply_port) {
+                                    Dart_Port reply_port,
+                                    char** error) {
   // Keep in sync with "sdk/lib/vmservice/vmservice.dart:_handleNativeRpcCall".
   Dart_CObject opcode;
   opcode.type = Dart_CObject_kInt32;
@@ -190,7 +192,25 @@ bool ServiceIsolate::SendServiceRpc(uint8_t* request_json,
   request.value.as_array.length = ARRAY_SIZE(request_array);
 
   ServiceIsolate::WaitForLoadPortInternal();
-  return Dart_PostCObject(ServiceIsolate::Port(), &request);
+  Dart_Port service_port = ServiceIsolate::Port();
+
+  const bool success = Dart_PostCObject(service_port, &request);
+
+  if (!success && error != nullptr) {
+    if (service_port == ILLEGAL_PORT) {
+      if (startup_failure_reason_ != nullptr) {
+        *error = OS::SCreate(/*zone=*/nullptr,
+                             "Service isolate failed to start up: %s.",
+                             startup_failure_reason_);
+      } else {
+        *error = strdup("No service isolate port was found.");
+      }
+    } else {
+      *error = strdup("Was unable to post message to service isolate.");
+    }
+  }
+
+  return success;
 }
 
 bool ServiceIsolate::SendIsolateStartupMessage() {
@@ -318,10 +338,11 @@ void ServiceIsolate::FinishedInitializing() {
   ml.NotifyAll();
 }
 
-void ServiceIsolate::InitializingFailed() {
+void ServiceIsolate::InitializingFailed(char* error) {
   MonitorLocker ml(monitor_);
   ASSERT(state_ == kStarting);
   state_ = kStopped;
+  startup_failure_reason_ = error;
   ml.NotifyAll();
 }
 
@@ -350,10 +371,15 @@ class RunServiceTask : public ThreadPool::Task {
                      ": Isolate creation error: %s\n",
                      error);
       }
+
+      char* formatted_error = OS::SCreate(
+          /*zone=*/nullptr, "Invoking the 'create_group' failed with: '%s'",
+          error);
+
       free(error);
       error = nullptr;
       ServiceIsolate::SetServiceIsolate(NULL);
-      ServiceIsolate::InitializingFailed();
+      ServiceIsolate::InitializingFailed(formatted_error);
       return;
     }
 
@@ -484,7 +510,8 @@ void ServiceIsolate::Run() {
   // that change this after Dart_Initialize returns.
   create_group_callback_ = Isolate::CreateGroupCallback();
   if (create_group_callback_ == NULL) {
-    ServiceIsolate::InitializingFailed();
+    ServiceIsolate::InitializingFailed(
+        strdup("The 'create_group' callback was not provided"));
     return;
   }
   bool task_started = Dart::thread_pool()->Run<RunServiceTask>();
@@ -547,6 +574,11 @@ void ServiceIsolate::Shutdown() {
     free(server_address_);
     server_address_ = NULL;
   }
+
+  if (startup_failure_reason_ != nullptr) {
+    free(startup_failure_reason_);
+    startup_failure_reason_ = nullptr;
+  }
 }
 
 void ServiceIsolate::BootVmServiceLibrary() {

runtime/vm/service_isolate.h

@@ -32,9 +32,14 @@ class ServiceIsolate : public AllStatic {
 
   // Returns `true` if the request was sucessfully sent.  If it was, the
   // [reply_port] will receive a Dart_TypedData_kUint8 response json.
+  //
+  // If sending the rpc failed and [error] is not `nullptr` then [error] might
+  // be set to a string containting the reason for the failure. If so, the
+  // caller is responsible for free()ing the error.
   static bool SendServiceRpc(uint8_t* request_json,
                              intptr_t request_json_length,
-                             Dart_Port reply_port);
+                             Dart_Port reply_port,
+                             char** error);
 
   static void Run();
   static bool SendIsolateStartupMessage();
@@ -66,7 +71,7 @@ class ServiceIsolate : public AllStatic {
   static void SetLoadPort(Dart_Port port);
   static void FinishedExiting();
   static void FinishedInitializing();
-  static void InitializingFailed();
+  static void InitializingFailed(char* error);
   static void MaybeMakeServiceIsolate(Isolate* isolate);
   static Dart_IsolateGroupCreateCallback create_group_callback() {
     return create_group_callback_;
@@ -87,6 +92,10 @@ class ServiceIsolate : public AllStatic {
   static Dart_Port origin_;
   static char* server_address_;
 
+  // If starting the service-isolate failed, this error might provide the reason
+  // for the failure.
+  static char* startup_failure_reason_;
+
   friend class Dart;
   friend class Isolate;
   friend class RunServiceTask;