From 28f692165ef9205c63af42f43b673b554314e321 Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Wed, 28 Aug 2019 13:18:52 +0000 Subject: [PATCH] [vm] Improve error message from Dart_InvokeVMServiceMethod() Change-Id: Ib7dad8a4e7fcc7ebb224bee848728e235f04459e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/114508 Commit-Queue: Martin Kustermann Reviewed-by: Clement Skau --- runtime/vm/native_api_impl.cc | 11 ++------- runtime/vm/service_isolate.cc | 44 ++++++++++++++++++++++++++++++----- runtime/vm/service_isolate.h | 13 +++++++++-- 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/runtime/vm/native_api_impl.cc b/runtime/vm/native_api_impl.cc index 2739b4369ad..191f485b1d1 100644 --- a/runtime/vm/native_api_impl.cc +++ b/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; } } diff --git a/runtime/vm/service_isolate.cc b/runtime/vm/service_isolate.cc index a0206b502a4..b01182be8a3 100644 --- a/runtime/vm/service_isolate.cc +++ b/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(); @@ -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() { diff --git a/runtime/vm/service_isolate.h b/runtime/vm/service_isolate.h index 25cef69bee2..a3c1fdfd11c 100644 --- a/runtime/vm/service_isolate.h +++ b/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;