Skip to content

Commit

Permalink
Support cleaner Dart isolate shutdown handling. (flutter#4121)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ianloic authored Sep 19, 2017
1 parent 773dfb5 commit dd1e0b5
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 5 deletions.
4 changes: 4 additions & 0 deletions lib/ui/ui_dart_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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(); }
Expand Down
6 changes: 6 additions & 0 deletions runtime/dart_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -217,4 +219,8 @@ void DartController::CreateIsolateFor(
Dart_ExitIsolate();
}

void DartController::IsolateShuttingDown() {
ui_dart_state_ = nullptr;
}

} // namespace blink
2 changes: 2 additions & 0 deletions runtime/dart_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class DartController {

UIDartState* dart_state() const { return ui_dart_state_; }

void IsolateShuttingDown();

private:
bool SendStartMessage(Dart_Handle root_library);

Expand Down
17 changes: 12 additions & 5 deletions runtime/dart_init.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<tonic::DartState*>(callback_data);
UIDartState* dart_state = static_cast<UIDartState*>(callback_data);
IsolateClient* isolate_client = dart_state->isolate_client();
if (isolate_client) {
isolate_client->WillShutDownIsolate(dart_state->isolate());
}
delete dart_state;
}

Expand Down Expand Up @@ -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<tonic::DartState*>(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(
Expand Down
4 changes: 4 additions & 0 deletions runtime/runtime_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions runtime/runtime_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class RuntimeController : public WindowClient, public IsolateClient {
void HandlePlatformMessage(fxl::RefPtr<PlatformMessage> message) override;

void DidCreateSecondaryIsolate(Dart_Isolate isolate) override;
void WillShutDownIsolate(Dart_Isolate isolate) override;

RuntimeDelegate* client_;
std::string language_code_;
Expand Down

0 comments on commit dd1e0b5

Please sign in to comment.