From e078d4f00a83a7e7418e2cd4e130822830b45d60 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Mon, 5 Oct 2020 20:42:02 +0000 Subject: [PATCH] [vm] Avoid allocations when reporting unhandled exceptions. Bug: https://github.com/dart-lang/sdk/issues/43642 Bug: b/169880355 Change-Id: I260b9d47f2b65d3cb4a48b966557d139978947e2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/165740 Commit-Queue: Ryan Macnak Reviewed-by: Martin Kustermann --- runtime/vm/isolate.cc | 61 +++++++++++++------ runtime/vm/isolate.h | 2 +- runtime/vm/object_store.cc | 19 ++---- ...ut_of_memory_unhandled_exception_test.dart | 36 +++++++++++ tests/standalone/standalone_kernel.status | 2 +- ...ut_of_memory_unhandled_exception_test.dart | 36 +++++++++++ tests/standalone_2/standalone_2_kernel.status | 1 + 7 files changed, 121 insertions(+), 36 deletions(-) create mode 100644 tests/standalone/out_of_memory_unhandled_exception_test.dart create mode 100644 tests/standalone_2/out_of_memory_unhandled_exception_test.dart diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index 8f475f60f84..5aa297b762c 100644 --- a/runtime/vm/isolate.cc +++ b/runtime/vm/isolate.cc @@ -133,6 +133,12 @@ static std::unique_ptr SerializeMessage(Dart_Port dest_port, } } +static std::unique_ptr SerializeMessage(Dart_Port dest_port, + Dart_CObject* obj) { + ApiMessageWriter writer; + return writer.WriteCMessage(obj, dest_port, Message::kNormalPriority); +} + static InstancePtr DeserializeMessage(Thread* thread, Message* message) { if (message == NULL) { return Instance::null(); @@ -1447,34 +1453,38 @@ MessageHandler::MessageStatus IsolateMessageHandler::ProcessUnhandledException( NoReloadScope no_reload_scope(T->isolate(), T); // Generate the error and stacktrace strings for the error message. - String& exc_str = String::Handle(T->zone()); - String& stacktrace_str = String::Handle(T->zone()); + const char* exception_cstr = nullptr; + const char* stacktrace_cstr = nullptr; if (result.IsUnhandledException()) { Zone* zone = T->zone(); const UnhandledException& uhe = UnhandledException::Cast(result); const Instance& exception = Instance::Handle(zone, uhe.exception()); - Object& tmp = Object::Handle(zone); - tmp = DartLibraryCalls::ToString(exception); - if (!tmp.IsString()) { - tmp = String::New(exception.ToCString()); + if (exception.raw() == I->object_store()->out_of_memory()) { + exception_cstr = "Out of Memory"; // Cf. OutOfMemoryError.toString(). + } else if (exception.raw() == I->object_store()->stack_overflow()) { + exception_cstr = "Stack Overflow"; // Cf. StackOverflowError.toString(). + } else { + const Object& exception_str = + Object::Handle(zone, DartLibraryCalls::ToString(exception)); + if (!exception_str.IsString()) { + exception_cstr = exception.ToCString(); + } else { + exception_cstr = exception_str.ToCString(); + } } - exc_str ^= tmp.raw(); const Instance& stacktrace = Instance::Handle(zone, uhe.stacktrace()); - tmp = DartLibraryCalls::ToString(stacktrace); - if (!tmp.IsString()) { - tmp = String::New(stacktrace.ToCString()); - } - stacktrace_str ^= tmp.raw(); + stacktrace_cstr = stacktrace.ToCString(); } else { - exc_str = String::New(result.ToErrorCString()); + exception_cstr = result.ToErrorCString(); } if (result.IsUnwindError()) { // When unwinding we don't notify error listeners and we ignore // whether errors are fatal for the current isolate. return StoreError(T, result); } else { - bool has_listener = I->NotifyErrorListeners(exc_str, stacktrace_str); + bool has_listener = + I->NotifyErrorListeners(exception_cstr, stacktrace_cstr); if (I->ErrorsFatal()) { if (has_listener) { T->ClearStickyError(); @@ -2276,21 +2286,32 @@ void Isolate::RemoveErrorListener(const SendPort& listener) { } } -bool Isolate::NotifyErrorListeners(const String& msg, - const String& stacktrace) { +bool Isolate::NotifyErrorListeners(const char* message, + const char* stacktrace) { const GrowableObjectArray& listeners = GrowableObjectArray::Handle( current_zone(), isolate_object_store()->error_listeners()); if (listeners.IsNull()) return false; - const Array& arr = Array::Handle(current_zone(), Array::New(2)); - arr.SetAt(0, msg); - arr.SetAt(1, stacktrace); + Dart_CObject arr; + Dart_CObject* arr_values[2]; + arr.type = Dart_CObject_kArray; + arr.value.as_array.length = 2; + arr.value.as_array.values = arr_values; + Dart_CObject msg; + msg.type = Dart_CObject_kString; + msg.value.as_string = const_cast(message); + arr_values[0] = &msg; + Dart_CObject stack; + stack.type = Dart_CObject_kString; + stack.value.as_string = const_cast(stacktrace); + arr_values[1] = &stack; + SendPort& listener = SendPort::Handle(current_zone()); for (intptr_t i = 0; i < listeners.Length(); i++) { listener ^= listeners.At(i); if (!listener.IsNull()) { Dart_Port port_id = listener.Id(); - PortMap::PostMessage(SerializeMessage(port_id, arr)); + PortMap::PostMessage(SerializeMessage(port_id, &arr)); } } return listeners.Length() > 0; diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index 66ff4ec81f0..51d50d9129a 100644 --- a/runtime/vm/isolate.h +++ b/runtime/vm/isolate.h @@ -1004,7 +1004,7 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry { void AddErrorListener(const SendPort& listener); void RemoveErrorListener(const SendPort& listener); - bool NotifyErrorListeners(const String& msg, const String& stacktrace); + bool NotifyErrorListeners(const char* msg, const char* stacktrace); bool ErrorsFatal() const { return ErrorsFatalBit::decode(isolate_flags_); } void SetErrorsFatal(bool val) { diff --git a/runtime/vm/object_store.cc b/runtime/vm/object_store.cc index 103f4e838b3..36705f7d0b2 100644 --- a/runtime/vm/object_store.cc +++ b/runtime/vm/object_store.cc @@ -55,17 +55,6 @@ void IsolateObjectStore::PrintToJSONObject(JSONObject* jsobj) { } #endif // !PRODUCT -static UnhandledExceptionPtr CreatePreallocatedUnandledException( - Zone* zone, - const Object& out_of_memory) { - // Allocate pre-allocated unhandled exception object initialized with the - // pre-allocated OutOfMemoryError. - const UnhandledException& unhandled_exception = - UnhandledException::Handle(UnhandledException::New( - Instance::Cast(out_of_memory), StackTrace::Handle(zone))); - return unhandled_exception.raw(); -} - static StackTracePtr CreatePreallocatedStackTrace(Zone* zone) { const Array& code_array = Array::Handle( zone, Array::New(StackTrace::kPreallocatedStackdepth, Heap::kOld)); @@ -93,10 +82,12 @@ ErrorPtr IsolateObjectStore::PreallocateObjects() { // pre-allocated OutOfMemoryError. const Object& out_of_memory = Object::Handle(zone, object_store_->out_of_memory()); + const StackTrace& preallocated_stack_trace = + StackTrace::Handle(zone, CreatePreallocatedStackTrace(zone)); + set_preallocated_stack_trace(preallocated_stack_trace); set_preallocated_unhandled_exception(UnhandledException::Handle( - CreatePreallocatedUnandledException(zone, out_of_memory))); - set_preallocated_stack_trace( - StackTrace::Handle(CreatePreallocatedStackTrace(zone))); + zone, UnhandledException::New(Instance::Cast(out_of_memory), + preallocated_stack_trace))); return Error::null(); } diff --git a/tests/standalone/out_of_memory_unhandled_exception_test.dart b/tests/standalone/out_of_memory_unhandled_exception_test.dart new file mode 100644 index 00000000000..7a5935d1175 --- /dev/null +++ b/tests/standalone/out_of_memory_unhandled_exception_test.dart @@ -0,0 +1,36 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import "dart:io"; + +import "package:expect/expect.dart"; + +main(args) async { + if (args.contains("--child")) { + var leak = []; + while (true) { + leak = [leak]; + } + } else { + var exec = Platform.resolvedExecutable; + var args = []; + args.addAll(Platform.executableArguments); + args.add("--old_gen_heap_size=20"); + args.add(Platform.script.toFilePath()); + args.add("--child"); + + // Should report an unhandled out of memory exception without crashing. + print("+ $exec " + args.join(" ")); + var result = await Process.run(exec, args); + + print("exit: ${result.exitCode}"); + print("stdout:"); + print(result.stdout); + print("stderr:"); + print(result.stderr); + + Expect.equals(255, result.exitCode, "Unhandled exception, not crash"); + Expect.isTrue(result.stderr.contains("Out of Memory")); + } +} diff --git a/tests/standalone/standalone_kernel.status b/tests/standalone/standalone_kernel.status index 199fb5aa4b5..02cfb78b27b 100644 --- a/tests/standalone/standalone_kernel.status +++ b/tests/standalone/standalone_kernel.status @@ -4,9 +4,9 @@ # Sections in this file should contain "$compiler == dartk" or # "$compiler == dartkp". -fragmentation_test: Pass, Slow # GC heavy fragmentation_typed_data_test: Pass, Slow # GC heavy io/process_sync_test: Pass, Slow # Spawns synchronously subprocesses in sequence. +out_of_memory_unhandled_exception_test: Pass, Slow [ $compiler == dartkb ] no_lazy_dispatchers_test: SkipByDesign # KBC interpreter doesn't support --no_lazy_dispatchers diff --git a/tests/standalone_2/out_of_memory_unhandled_exception_test.dart b/tests/standalone_2/out_of_memory_unhandled_exception_test.dart new file mode 100644 index 00000000000..7a5935d1175 --- /dev/null +++ b/tests/standalone_2/out_of_memory_unhandled_exception_test.dart @@ -0,0 +1,36 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import "dart:io"; + +import "package:expect/expect.dart"; + +main(args) async { + if (args.contains("--child")) { + var leak = []; + while (true) { + leak = [leak]; + } + } else { + var exec = Platform.resolvedExecutable; + var args = []; + args.addAll(Platform.executableArguments); + args.add("--old_gen_heap_size=20"); + args.add(Platform.script.toFilePath()); + args.add("--child"); + + // Should report an unhandled out of memory exception without crashing. + print("+ $exec " + args.join(" ")); + var result = await Process.run(exec, args); + + print("exit: ${result.exitCode}"); + print("stdout:"); + print(result.stdout); + print("stderr:"); + print(result.stderr); + + Expect.equals(255, result.exitCode, "Unhandled exception, not crash"); + Expect.isTrue(result.stderr.contains("Out of Memory")); + } +} diff --git a/tests/standalone_2/standalone_2_kernel.status b/tests/standalone_2/standalone_2_kernel.status index 199fb5aa4b5..af8e3ec4586 100644 --- a/tests/standalone_2/standalone_2_kernel.status +++ b/tests/standalone_2/standalone_2_kernel.status @@ -7,6 +7,7 @@ fragmentation_test: Pass, Slow # GC heavy fragmentation_typed_data_test: Pass, Slow # GC heavy io/process_sync_test: Pass, Slow # Spawns synchronously subprocesses in sequence. +out_of_memory_unhandled_exception_test: Pass, Slow [ $compiler == dartkb ] no_lazy_dispatchers_test: SkipByDesign # KBC interpreter doesn't support --no_lazy_dispatchers