From 06a471ce86a89a7d712232381a664e6469afaab8 Mon Sep 17 00:00:00 2001 From: asiva Date: Fri, 27 Aug 2021 20:49:39 +0000 Subject: [PATCH] [VM/Runtime] - Return error instead of a FATAL error when failing to communicate with the kernel isolate while accepting compilation results during a hot reload. TEST=reload bot tests. Change-Id: I55f983cc8461c89e91bf1bef84f39dcda61e8142 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/210942 Reviewed-by: Alexander Aprelev Commit-Queue: Siva Annamalai --- runtime/bin/dfe.cc | 1 + runtime/include/dart_api.h | 1 + runtime/vm/isolate_reload.cc | 44 +++++++++++++++++++++++++++--------- runtime/vm/kernel_isolate.cc | 14 ++++++------ 4 files changed, 42 insertions(+), 18 deletions(-) diff --git a/runtime/bin/dfe.cc b/runtime/bin/dfe.cc index a5710f148f2..b6c27fa9ddc 100644 --- a/runtime/bin/dfe.cc +++ b/runtime/bin/dfe.cc @@ -221,6 +221,7 @@ void DFE::CompileAndReadScript(const char* script_uri, *exit_code = kDartFrontendErrorExitCode; break; case Dart_KernelCompilationStatus_Unknown: + case Dart_KernelCompilationStatus_MsgFailed: free(result.kernel); *error = result.error; // Copy error message. *exit_code = kErrorExitCode; diff --git a/runtime/include/dart_api.h b/runtime/include/dart_api.h index a328eb749b6..a753a56ba37 100644 --- a/runtime/include/dart_api.h +++ b/runtime/include/dart_api.h @@ -3538,6 +3538,7 @@ typedef enum { Dart_KernelCompilationStatus_Ok = 0, Dart_KernelCompilationStatus_Error = 1, Dart_KernelCompilationStatus_Crash = 2, + Dart_KernelCompilationStatus_MsgFailed = 3, } Dart_KernelCompilationStatus; typedef struct { diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc index 999790d1013..33a86d0e95f 100644 --- a/runtime/vm/isolate_reload.cc +++ b/runtime/vm/isolate_reload.cc @@ -530,15 +530,26 @@ static intptr_t CommonSuffixLength(const char* a, const char* b) { return (a_length - a_cursor); } -static void AcceptCompilation(Thread* thread) { +static ObjectPtr AcceptCompilation(Thread* thread) { TransitionVMToNative transition(thread); Dart_KernelCompilationResult result = KernelIsolate::AcceptCompilation(); if (result.status != Dart_KernelCompilationStatus_Ok) { - FATAL1( - "An error occurred in the CFE while accepting the most recent" + if (result.status != Dart_KernelCompilationStatus_MsgFailed) { + FATAL1( + "An error occurred while accepting the most recent" + " compilation results: %s", + result.error); + } + TIR_Print( + "An error occurred while accepting the most recent" " compilation results: %s", result.error); + Zone* zone = thread->zone(); + const auto& error_str = String::Handle(zone, String::New(result.error)); + free(result.error); + return ApiError::New(error_str); } + return Object::null(); } // If [root_script_url] is null, attempt to load from [kernel_buffer]. @@ -639,7 +650,14 @@ bool IsolateGroupReloadContext::Reload(bool force_reload, // we have accepted the compilation to clear some state in the incremental // compiler. if (did_kernel_compilation) { - AcceptCompilation(thread); + const auto& result = Object::Handle(Z, AcceptCompilation(thread)); + if (result.IsError()) { + const auto& error = Error::Cast(result); + AddReasonForCancelling(new Aborted(Z, error)); + ReportReasonsForCancelling(); + CommonFinalizeTail(num_old_libs_); + return false; + } } TIR_Print("---- SKIPPING RELOAD (No libraries were modified)\n"); return false; @@ -739,6 +757,17 @@ bool IsolateGroupReloadContext::Reload(bool force_reload, heap->CollectAllGarbage(Heap::kLowMemory); } + // If we use the CFE and performed a compilation, we need to notify that + // we have accepted the compilation to clear some state in the incremental + // compiler. + if (did_kernel_compilation) { + const auto& result = Object::Handle(Z, AcceptCompilation(thread)); + if (result.IsError()) { + const auto& error = Error::Cast(result); + AddReasonForCancelling(new Aborted(Z, error)); + } + } + if (!FLAG_reload_force_rollback && !HasReasonsForCancelling()) { TIR_Print("---- COMMITTING RELOAD\n"); isolate_group_->program_reload_context()->ReloadPhase4CommitPrepare(); @@ -828,13 +857,6 @@ bool IsolateGroupReloadContext::Reload(bool force_reload, GrowableObjectArray::Handle(Z, IG->object_store()->libraries()) .Length(); CommonFinalizeTail(final_library_count); - - // If we use the CFE and performed a compilation, we need to notify that - // we have accepted the compilation to clear some state in the incremental - // compiler. - if (did_kernel_compilation) { - AcceptCompilation(thread); - } } // Reenable concurrent marking if it was initially on. diff --git a/runtime/vm/kernel_isolate.cc b/runtime/vm/kernel_isolate.cc index f8b17ab41cc..4156252e4b3 100644 --- a/runtime/vm/kernel_isolate.cc +++ b/runtime/vm/kernel_isolate.cc @@ -484,7 +484,7 @@ class KernelCompilationRequest : public ValueObject { const MallocGrowableArray* experimental_flags) { if (port_ == ILLEGAL_PORT) { Dart_KernelCompilationResult result = {}; - result.status = Dart_KernelCompilationStatus_Unknown; + result.status = Dart_KernelCompilationStatus_MsgFailed; result.error = Utils::StrDup("Error Kernel Isolate : unable to create reply port"); return result; @@ -719,7 +719,7 @@ class KernelCompilationRequest : public ValueObject { // tag is used to specify which operation the frontend should perform. if (port_ == ILLEGAL_PORT) { Dart_KernelCompilationResult result = {}; - result.status = Dart_KernelCompilationStatus_Unknown; + result.status = Dart_KernelCompilationStatus_MsgFailed; result.error = Utils::StrDup("Error Kernel Isolate : unable to create reply port"); return result; @@ -1057,7 +1057,7 @@ Dart_KernelCompilationResult KernelIsolate::CompileToKernel( Dart_Port kernel_port = WaitForKernelPort(); if (kernel_port == ILLEGAL_PORT) { Dart_KernelCompilationResult result = {}; - result.status = Dart_KernelCompilationStatus_Unknown; + result.status = Dart_KernelCompilationStatus_MsgFailed; result.error = Utils::StrDup("Error while initializing Kernel isolate"); return result; } @@ -1095,7 +1095,7 @@ Dart_KernelCompilationResult KernelIsolate::ListDependencies() { Dart_Port kernel_port = WaitForKernelPort(); if (kernel_port == ILLEGAL_PORT) { Dart_KernelCompilationResult result = {}; - result.status = Dart_KernelCompilationStatus_Unknown; + result.status = Dart_KernelCompilationStatus_MsgFailed; result.error = Utils::StrDup("Error while initializing Kernel isolate"); return result; } @@ -1113,7 +1113,7 @@ Dart_KernelCompilationResult KernelIsolate::AcceptCompilation() { Dart_Port kernel_port = WaitForKernelPort(); if (kernel_port == ILLEGAL_PORT) { Dart_KernelCompilationResult result = {}; - result.status = Dart_KernelCompilationStatus_Unknown; + result.status = Dart_KernelCompilationStatus_MsgFailed; result.error = Utils::StrDup("Error while initializing Kernel isolate"); return result; } @@ -1137,7 +1137,7 @@ Dart_KernelCompilationResult KernelIsolate::CompileExpressionToKernel( Dart_Port kernel_port = WaitForKernelPort(); if (kernel_port == ILLEGAL_PORT) { Dart_KernelCompilationResult result = {}; - result.status = Dart_KernelCompilationStatus_Unknown; + result.status = Dart_KernelCompilationStatus_MsgFailed; result.error = Utils::StrDup("Error while initializing Kernel isolate"); return result; } @@ -1159,7 +1159,7 @@ Dart_KernelCompilationResult KernelIsolate::UpdateInMemorySources( Dart_Port kernel_port = WaitForKernelPort(); if (kernel_port == ILLEGAL_PORT) { Dart_KernelCompilationResult result = {}; - result.status = Dart_KernelCompilationStatus_Unknown; + result.status = Dart_KernelCompilationStatus_MsgFailed; result.error = Utils::StrDup("Error while initializing Kernel isolate"); return result; }