Skip to content

Commit

Permalink
Reland children isolates sharing isolate group change. (flutter#13758)
Browse files Browse the repository at this point in the history 
* Revert "Revert "Provide dart vm initalize isolate callback so that children isolates belong to parent's isolate group. (flutter#9888)" (flutter#12327)"

* Ensure that when isolate shuts down it calls isolate_data, rather than isolage_group_data callback.
  • Loading branch information
@aam
aam authored Nov 15, 2019
1 parent cc2fe6b commit 096ba68
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 46 deletions.
179 changes: 140 additions & 39 deletions runtime/dart_isolate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
advisory_script_entrypoint, // advisory entrypoint
nullptr, // child isolate preparer
isolate_create_callback, // isolate create callback
isolate_shutdown_callback // isolate shutdown callback
isolate_shutdown_callback, // isolate shutdown callback
true, // is_root_isolate
true // is_group_root_isolate
)));

std::tie(vm_isolate, embedder_isolate) = CreateDartVMAndEmbedderObjectPair(
Expand Down Expand Up @@ -113,7 +115,9 @@ DartIsolate::DartIsolate(const Settings& settings,
std::string advisory_script_entrypoint,
ChildIsolatePreparer child_isolate_preparer,
fml::closure isolate_create_callback,
fml::closure isolate_shutdown_callback)
fml::closure isolate_shutdown_callback,
bool is_root_isolate,
bool is_group_root_isolate)
: UIDartState(std::move(task_runners),
settings.task_observer_add,
settings.task_observer_remove,
Expand All @@ -130,7 +134,9 @@ DartIsolate::DartIsolate(const Settings& settings,
isolate_snapshot_(std::move(isolate_snapshot)),
child_isolate_preparer_(std::move(child_isolate_preparer)),
isolate_create_callback_(isolate_create_callback),
isolate_shutdown_callback_(isolate_shutdown_callback) {
isolate_shutdown_callback_(isolate_shutdown_callback),
is_root_isolate_(is_root_isolate),
is_group_root_isolate_(is_group_root_isolate) {
FML_DCHECK(isolate_snapshot_) << "Must contain a valid isolate snapshot.";
phase_ = Phase::Uninitialized;
}
Expand All @@ -152,7 +158,7 @@ std::string DartIsolate::GetServiceId() {
return service_id;
}

bool DartIsolate::Initialize(Dart_Isolate dart_isolate, bool is_root_isolate) {
bool DartIsolate::Initialize(Dart_Isolate dart_isolate) {
TRACE_EVENT0("flutter", "DartIsolate::Initialize");
if (phase_ != Phase::Uninitialized) {
return false;
Expand All @@ -166,12 +172,6 @@ bool DartIsolate::Initialize(Dart_Isolate dart_isolate, bool is_root_isolate) {
return false;
}

auto* isolate_data = static_cast<std::shared_ptr<DartIsolate>*>(
Dart_IsolateGroupData(dart_isolate));
if (isolate_data->get() != this) {
return false;
}

// After this point, isolate scopes can be safely used.
SetIsolate(dart_isolate);

Expand All @@ -183,8 +183,7 @@ bool DartIsolate::Initialize(Dart_Isolate dart_isolate, bool is_root_isolate) {

tonic::DartIsolateScope scope(isolate());

SetMessageHandlingTaskRunner(GetTaskRunners().GetUITaskRunner(),
is_root_isolate);
SetMessageHandlingTaskRunner(GetTaskRunners().GetUITaskRunner());

if (tonic::LogIfError(
Dart_SetLibraryTagHandler(tonic::DartState::HandleLibraryTag))) {
Expand All @@ -204,9 +203,8 @@ fml::RefPtr<fml::TaskRunner> DartIsolate::GetMessageHandlingTaskRunner() const {
}

void DartIsolate::SetMessageHandlingTaskRunner(
fml::RefPtr<fml::TaskRunner> runner,
bool is_root_isolate) {
if (!is_root_isolate || !runner) {
fml::RefPtr<fml::TaskRunner> runner) {
if (!IsRootIsolate() || !runner) {
return;
}

Expand Down Expand Up @@ -255,7 +253,7 @@ bool DartIsolate::UpdateThreadPoolNames() const {
return true;
}

bool DartIsolate::LoadLibraries(bool is_root_isolate) {
bool DartIsolate::LoadLibraries() {
TRACE_EVENT0("flutter", "DartIsolate::LoadLibraries");
if (phase_ != Phase::Initialized) {
return false;
Expand All @@ -265,11 +263,11 @@ bool DartIsolate::LoadLibraries(bool is_root_isolate) {

DartIO::InitForIsolate();

DartUI::InitForIsolate(is_root_isolate);
DartUI::InitForIsolate(IsRootIsolate());

const bool is_service_isolate = Dart_IsServiceIsolate(isolate());

DartRuntimeHooks::Install(is_root_isolate && !is_service_isolate,
DartRuntimeHooks::Install(IsRootIsolate() && !is_service_isolate,
GetAdvisoryScriptURI());

if (!is_service_isolate) {
Expand Down Expand Up @@ -650,6 +648,7 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
Dart_IsolateFlags* flags,
std::shared_ptr<DartIsolate>* parent_embedder_isolate,
char** error) {
TRACE_EVENT0("flutter", "DartIsolate::DartIsolateGroupCreateCallback");
if (parent_embedder_isolate == nullptr &&
strcmp(advisory_script_uri, DART_VM_SERVICE_ISOLATE_NAME) == 0) {
// The VM attempts to start the VM service for us on |Dart_Initialize|. In
Expand Down Expand Up @@ -677,6 +676,59 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
.first;
}

// |Dart_IsolateInitializeCallback|
bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data,
char** error) {
TRACE_EVENT0("flutter", "DartIsolate::DartIsolateInitializeCallback");
Dart_Isolate isolate = Dart_CurrentIsolate();
if (isolate == nullptr) {
*error = strdup("Isolate should be available in initialize callback.");
FML_DLOG(ERROR) << *error;
return false;
}

auto* root_embedder_isolate = static_cast<std::shared_ptr<DartIsolate>*>(
Dart_CurrentIsolateGroupData());

TaskRunners null_task_runners(
(*root_embedder_isolate)->GetAdvisoryScriptURI(),
/* platform= */ nullptr, /* gpu= */ nullptr,
/* ui= */ nullptr,
/* io= */ nullptr);

auto embedder_isolate = std::make_unique<std::shared_ptr<DartIsolate>>(
std::shared_ptr<DartIsolate>(new DartIsolate(
(*root_embedder_isolate)->GetSettings(), // settings
(*root_embedder_isolate)->GetIsolateSnapshot(), // isolate_snapshot
null_task_runners, // task_runners
fml::WeakPtr<SnapshotDelegate>{}, // snapshot_delegate
fml::WeakPtr<IOManager>{}, // io_manager
fml::RefPtr<SkiaUnrefQueue>{}, // unref_queue
fml::WeakPtr<ImageDecoder>{}, // image_decoder
(*root_embedder_isolate)
->GetAdvisoryScriptURI(), // advisory_script_uri
(*root_embedder_isolate)
->GetAdvisoryScriptEntrypoint(), // advisory_script_entrypoint
(*root_embedder_isolate)->child_isolate_preparer_, // preparer
(*root_embedder_isolate)->isolate_create_callback_, // on create
(*root_embedder_isolate)->isolate_shutdown_callback_, // on shutdown
false, // is_root_isolate
false))); // is_group_root_isolate

// root isolate should have been created via CreateRootIsolate and
// CreateDartVMAndEmbedderObjectPair
if (!InitializeIsolate(*embedder_isolate, isolate, error)) {
return false;
}

// The ownership of the embedder object is controlled by the Dart VM. So the
// only reference returned to the caller is weak.
*child_callback_data = embedder_isolate.release();

Dart_EnterIsolate(isolate);
return true;
}

std::pair<Dart_Isolate, std::weak_ptr<DartIsolate>>
DartIsolate::CreateDartVMAndEmbedderObjectPair(
const char* advisory_script_uri,
Expand Down Expand Up @@ -717,11 +769,11 @@ DartIsolate::CreateDartVMAndEmbedderObjectPair(
fml::WeakPtr<ImageDecoder>{}, // image_decoder
advisory_script_uri, // advisory_script_uri
advisory_script_entrypoint, // advisory_script_entrypoint
(*raw_embedder_isolate)->child_isolate_preparer_, // preparer
(*raw_embedder_isolate)->isolate_create_callback_, // on create
(*raw_embedder_isolate)->isolate_shutdown_callback_ // on shutdown
))

(*raw_embedder_isolate)->child_isolate_preparer_, // preparer
(*raw_embedder_isolate)->isolate_create_callback_, // on create
(*raw_embedder_isolate)->isolate_shutdown_callback_, // on shutdown
is_root_isolate,
true)) // is_root_group_isolate
);
}

Expand All @@ -741,38 +793,53 @@ DartIsolate::CreateDartVMAndEmbedderObjectPair(
return {nullptr, {}};
}

if (!(*embedder_isolate)->Initialize(isolate, is_root_isolate)) {
if (!InitializeIsolate(*embedder_isolate, isolate, error)) {
return {nullptr, {}};
}

auto* isolate_data = static_cast<std::shared_ptr<DartIsolate>*>(
Dart_IsolateGroupData(isolate));
FML_DCHECK(isolate_data->get() == embedder_isolate->get());

auto weak_embedder_isolate = (*embedder_isolate)->GetWeakIsolatePtr();

// The ownership of the embedder object is controlled by the Dart VM. So the
// only reference returned to the caller is weak.
embedder_isolate.release();
return {isolate, weak_embedder_isolate};
}

bool DartIsolate::InitializeIsolate(
std::shared_ptr<DartIsolate> embedder_isolate,
Dart_Isolate isolate,
char** error) {
TRACE_EVENT0("flutter", "DartIsolate::InitializeIsolate");
if (!embedder_isolate->Initialize(isolate)) {
*error = strdup("Embedder could not initialize the Dart isolate.");
FML_DLOG(ERROR) << *error;
return {nullptr, {}};
return false;
}

if (!(*embedder_isolate)->LoadLibraries(is_root_isolate)) {
if (!embedder_isolate->LoadLibraries()) {
*error =
strdup("Embedder could not load libraries in the new Dart isolate.");
FML_DLOG(ERROR) << *error;
return {nullptr, {}};
return false;
}

auto weak_embedder_isolate = (*embedder_isolate)->GetWeakIsolatePtr();

// Root isolates will be setup by the engine and the service isolate (which is
// also a root isolate) by the utility routines in the VM. However, secondary
// isolates will be run by the VM if they are marked as runnable.
if (!is_root_isolate) {
FML_DCHECK((*embedder_isolate)->child_isolate_preparer_);
if (!(*embedder_isolate)
->child_isolate_preparer_((*embedder_isolate).get())) {
if (!embedder_isolate->IsRootIsolate()) {
FML_DCHECK(embedder_isolate->child_isolate_preparer_);
if (!embedder_isolate->child_isolate_preparer_(embedder_isolate.get())) {
*error = strdup("Could not prepare the child isolate to run.");
FML_DLOG(ERROR) << *error;
return {nullptr, {}};
return false;
}
}

// The ownership of the embedder object is controlled by the Dart VM. So the
// only reference returned to the caller is weak.
embedder_isolate.release();
return {isolate, weak_embedder_isolate};
return true;
}

// |Dart_IsolateShutdownCallback|
Expand All @@ -783,12 +850,46 @@ void DartIsolate::DartIsolateShutdownCallback(
FML_DLOG(INFO) << "DartIsolateShutdownCallback"
<< " isolate_group_data " << isolate_group_data
<< " isolate_data " << isolate_data;
isolate_group_data->get()->OnShutdownCallback();
isolate_data->get()->OnShutdownCallback();
}

// |Dart_IsolateGroupCleanupCallback|
void DartIsolate::DartIsolateGroupCleanupCallback(
std::shared_ptr<DartIsolate>* isolate_data) {
TRACE_EVENT0("flutter", "DartIsolate::DartIsolateGroupCleanupCallback");
FML_DLOG(INFO) << "DartIsolateGroupCleanupCallback isolate_data "
<< isolate_data;

delete isolate_data;
}

// |Dart_IsolateCleanupCallback|
void DartIsolate::DartIsolateCleanupCallback(
std::shared_ptr<DartIsolate>* isolate_group_data,
std::shared_ptr<DartIsolate>* isolate_data) {
TRACE_EVENT0("flutter", "DartIsolate::DartIsolateCleanupCallback");

if ((*isolate_data)->IsRootIsolate()) {
// isolate_data will be cleaned up as part of IsolateGroup cleanup
FML_DLOG(INFO)
<< "DartIsolateCleanupCallback no-op for root isolate isolate_data "
<< isolate_data;
return;
}
if ((*isolate_data)->IsGroupRootIsolate()) {
// Even if isolate was not a root isolate(i.e. was spawned),
// it might have IsolateGroup created for it (when
// --no-enable-isolate-groups dart vm flag is used).
// Then its isolate_data will be cleaned up as part of IsolateGroup
// cleanup as well.
FML_DLOG(INFO) << "DartIsolateCleanupCallback no-op for group root isolate "
"isolate_data "
<< isolate_data;
return;
}

FML_DLOG(INFO) << "DartIsolateCleanupCallback cleaned up isolate_data "
<< isolate_data;
delete isolate_data;
}

Expand Down
35 changes: 28 additions & 7 deletions runtime/dart_isolate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,13 @@ class DartIsolate : public UIDartState {
///
fml::RefPtr<fml::TaskRunner> GetMessageHandlingTaskRunner() const;

// Root isolate of the VM application
bool IsRootIsolate() const { return is_root_isolate_; }
// Isolate that owns IsolateGroup it lives in.
// When --no-enable-isolate-groups dart vm flag is set,
// all child isolates will have their own IsolateGroups.
bool IsGroupRootIsolate() const { return is_group_root_isolate_; }

private:
using ChildIsolatePreparer = std::function<bool(DartIsolate*)>;

Expand All @@ -425,6 +432,8 @@ class DartIsolate : public UIDartState {
fml::RefPtr<fml::TaskRunner> message_handling_task_runner_;
const fml::closure isolate_create_callback_;
const fml::closure isolate_shutdown_callback_;
const bool is_root_isolate_;
const bool is_group_root_isolate_;

DartIsolate(const Settings& settings,
fml::RefPtr<const DartSnapshot> isolate_snapshot,
Expand All @@ -437,18 +446,17 @@ class DartIsolate : public UIDartState {
std::string advisory_script_entrypoint,
ChildIsolatePreparer child_isolate_preparer,
fml::closure isolate_create_callback,
fml::closure isolate_shutdown_callback);

FML_WARN_UNUSED_RESULT bool Initialize(Dart_Isolate isolate,
bool is_root_isolate);
fml::closure isolate_shutdown_callback,
bool is_root_isolate,
bool is_group_root_isolate);
FML_WARN_UNUSED_RESULT bool Initialize(Dart_Isolate isolate);

void SetMessageHandlingTaskRunner(fml::RefPtr<fml::TaskRunner> runner,
bool is_root_isolate);
void SetMessageHandlingTaskRunner(fml::RefPtr<fml::TaskRunner> runner);

bool LoadKernel(std::shared_ptr<const fml::Mapping> mapping, bool last_piece);

FML_WARN_UNUSED_RESULT
bool LoadLibraries(bool is_root_isolate);
bool LoadLibraries();

bool UpdateThreadPoolNames() const;

Expand All @@ -467,6 +475,10 @@ class DartIsolate : public UIDartState {
std::shared_ptr<DartIsolate>* embedder_isolate,
char** error);

// |Dart_IsolateInitializeCallback|
static bool DartIsolateInitializeCallback(void** child_callback_data,
char** error);

static Dart_Isolate DartCreateAndStartServiceIsolate(
const char* package_root,
const char* package_config,
Expand All @@ -485,11 +497,20 @@ class DartIsolate : public UIDartState {
bool is_root_isolate,
char** error);

static bool InitializeIsolate(std::shared_ptr<DartIsolate> embedder_isolate,
Dart_Isolate isolate,
char** error);

// |Dart_IsolateShutdownCallback|
static void DartIsolateShutdownCallback(
std::shared_ptr<DartIsolate>* isolate_group_data,
std::shared_ptr<DartIsolate>* isolate_data);

// |Dart_IsolateCleanupCallback|
static void DartIsolateCleanupCallback(
std::shared_ptr<DartIsolate>* isolate_group_data,
std::shared_ptr<DartIsolate>* isolate_data);

// |Dart_IsolateGroupCleanupCallback|
static void DartIsolateGroupCleanupCallback(
std::shared_ptr<DartIsolate>* isolate_group_data);
Expand Down
Loading

0 comments on commit 096ba68

Please sign in to comment.