From dd1e0b59ece026737b05640cf020a6e590cfc10c Mon Sep 17 00:00:00 2001 From: Ian McKellar Date: Tue, 19 Sep 2017 12:11:05 -0700 Subject: [PATCH] Support cleaner Dart isolate shutdown handling. (#4121) If an isolate shuts down (for example if an app calls Isolate.current.kill()), the UIDartState* on DartController will refer to a freed object. This wires through notification that the is shutting down through to the DartController so it can clean up appropriately. This also makes gives the vm-service isolate an UIDartState* so that the shutdown callback can behave correctly. --- lib/ui/ui_dart_state.h | 4 ++++ runtime/dart_controller.cc | 6 ++++++ runtime/dart_controller.h | 2 ++ runtime/dart_init.cc | 17 ++++++++++++----- runtime/runtime_controller.cc | 4 ++++ runtime/runtime_controller.h | 1 + 6 files changed, 29 insertions(+), 5 deletions(-) diff --git a/lib/ui/ui_dart_state.h b/lib/ui/ui_dart_state.h index 8c5d9928b65d7..5b16b6379532d 100644 --- a/lib/ui/ui_dart_state.h +++ b/lib/ui/ui_dart_state.h @@ -20,6 +20,7 @@ class Window; class IsolateClient { public: virtual void DidCreateSecondaryIsolate(Dart_Isolate isolate) = 0; + virtual void WillShutDownIsolate(Dart_Isolate isolate) = 0; protected: virtual ~IsolateClient(); @@ -35,6 +36,9 @@ class UIDartState : public tonic::DartState { UIDartState* CreateForChildIsolate(); IsolateClient* isolate_client() { return isolate_client_; } + void set_isolate_client(IsolateClient* isolate_client) { + isolate_client_ = isolate_client; + } Dart_Port main_port() const { return main_port_; } const std::string& debug_name() const { return debug_name_; } Window* window() const { return window_.get(); } diff --git a/runtime/dart_controller.cc b/runtime/dart_controller.cc index 1fd2b5436a3e4..b9ece77ea8240 100644 --- a/runtime/dart_controller.cc +++ b/runtime/dart_controller.cc @@ -49,6 +49,8 @@ DartController::DartController() DartController::~DartController() { if (ui_dart_state_) { + ui_dart_state_->set_isolate_client(nullptr); + // Don't use a tonic::DartIsolateScope here since we never exit the isolate. Dart_EnterIsolate(ui_dart_state_->isolate()); // Clear the message notify callback. @@ -217,4 +219,8 @@ void DartController::CreateIsolateFor( Dart_ExitIsolate(); } +void DartController::IsolateShuttingDown() { + ui_dart_state_ = nullptr; +} + } // namespace blink diff --git a/runtime/dart_controller.h b/runtime/dart_controller.h index 0145d30089439..5706bca234f83 100644 --- a/runtime/dart_controller.h +++ b/runtime/dart_controller.h @@ -35,6 +35,8 @@ class DartController { UIDartState* dart_state() const { return ui_dart_state_; } + void IsolateShuttingDown(); + private: bool SendStartMessage(Dart_Handle root_library); diff --git a/runtime/dart_init.cc b/runtime/dart_init.cc index 9a28a803bd4e0..874fad3c984b0 100644 --- a/runtime/dart_init.cc +++ b/runtime/dart_init.cc @@ -24,6 +24,7 @@ #include "flutter/lib/ui/dart_runtime_hooks.h" #include "flutter/lib/ui/dart_ui.h" #include "flutter/lib/ui/ui_dart_state.h" +#include "flutter/lib/ui/window/window.h" #include "flutter/runtime/dart_service_isolate.h" #include "flutter/runtime/start_up.h" #include "lib/fxl/arraysize.h" @@ -129,7 +130,11 @@ void IsolateShutdownCallback(void* callback_data) { Dart_Handle sticky_error = Dart_GetStickyError(); FXL_CHECK(LogIfError(sticky_error)); } - tonic::DartState* dart_state = static_cast(callback_data); + UIDartState* dart_state = static_cast(callback_data); + IsolateClient* isolate_client = dart_state->isolate_client(); + if (isolate_client) { + isolate_client->WillShutDownIsolate(dart_state->isolate()); + } delete dart_state; } @@ -188,11 +193,13 @@ Dart_Isolate ServiceIsolateCreateCallback(const char* script_uri, // No VM-service in release mode. return nullptr; #else // FLUTTER_RUNTIME_MODE - tonic::DartState* dart_state = new tonic::DartState(); - Dart_Isolate isolate = Dart_CreateIsolate( - script_uri, "main", g_default_isolate_snapshot_data, - g_default_isolate_snapshot_instructions, nullptr, dart_state, error); + UIDartState* dart_state = new UIDartState(nullptr, nullptr); + Dart_Isolate isolate = + Dart_CreateIsolate(script_uri, "main", g_default_isolate_snapshot_data, + g_default_isolate_snapshot_instructions, nullptr, + static_cast(dart_state), error); FXL_CHECK(isolate) << error; + dart_state->set_debug_name_prefix(script_uri); dart_state->SetIsolate(isolate); FXL_CHECK(Dart_IsServiceIsolate(isolate)); FXL_CHECK(!LogIfError( diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 563c6b25c4655..ea6f430f32356 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -137,6 +137,10 @@ void RuntimeController::DidCreateSecondaryIsolate(Dart_Isolate isolate) { client_->DidCreateSecondaryIsolate(isolate); } +void RuntimeController::WillShutDownIsolate(Dart_Isolate isolate) { + dart_controller_->IsolateShuttingDown(); +} + Dart_Port RuntimeController::GetMainPort() { if (!dart_controller_) { return ILLEGAL_PORT; diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 82032394bc0e4..a24f851c6c9ee 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -61,6 +61,7 @@ class RuntimeController : public WindowClient, public IsolateClient { void HandlePlatformMessage(fxl::RefPtr message) override; void DidCreateSecondaryIsolate(Dart_Isolate isolate) override; + void WillShutDownIsolate(Dart_Isolate isolate) override; RuntimeDelegate* client_; std::string language_code_;