Skip to content

Commit 5585ed9

Browse files
liyuqianliyuqian
authored
Revert "Create root isolate asynchronously (flutter#20142)" (flutter#20937)
This reverts commit 95f2b72.
1 parent 96efe39 commit 5585ed9

File tree

8 files changed

+56
-149
lines changed

8 files changed

+56
-149
lines changed

‎runtime/runtime_controller.cc

Lines changed: 49 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@ namespace flutter {
1818

1919
RuntimeController::RuntimeController(RuntimeDelegate& client,
2020
TaskRunners p_task_runners)
21-
: client_(client),
22-
vm_(nullptr),
23-
task_runners_(p_task_runners),
24-
weak_factory_(this) {}
21+
: client_(client), vm_(nullptr), task_runners_(p_task_runners) {}
2522

2623
RuntimeController::RuntimeController(
2724
RuntimeDelegate& p_client,
@@ -53,80 +50,48 @@ RuntimeController::RuntimeController(
5350
platform_data_(std::move(p_platform_data)),
5451
isolate_create_callback_(p_isolate_create_callback),
5552
isolate_shutdown_callback_(p_isolate_shutdown_callback),
56-
persistent_isolate_data_(std::move(p_persistent_isolate_data)),
57-
weak_factory_(this) {
58-
// Create the root isolate as soon as the runtime controller is initialized,
59-
// but not using a synchronous way to avoid blocking the platform thread a
60-
// long time as it is waiting while creating `Shell` on that platform thread.
53+
persistent_isolate_data_(std::move(p_persistent_isolate_data)) {
54+
// Create the root isolate as soon as the runtime controller is initialized.
6155
// It will be run at a later point when the engine provides a run
6256
// configuration and then runs the isolate.
63-
create_and_config_root_isolate_ =
64-
std::async(std::launch::deferred, [self = weak_factory_.GetWeakPtr()]() {
65-
if (!self) {
66-
return;
67-
}
68-
69-
auto strong_root_isolate =
70-
DartIsolate::CreateRootIsolate(
71-
self->vm_->GetVMData()->GetSettings(), //
72-
self->isolate_snapshot_, //
73-
self->task_runners_, //
74-
std::make_unique<PlatformConfiguration>(self.get()), //
75-
self->snapshot_delegate_, //
76-
self->io_manager_, //
77-
self->unref_queue_, //
78-
self->image_decoder_, //
79-
self->advisory_script_uri_, //
80-
self->advisory_script_entrypoint_, //
81-
nullptr, //
82-
self->isolate_create_callback_, //
83-
self->isolate_shutdown_callback_ //
84-
)
85-
.lock();
86-
87-
FML_CHECK(strong_root_isolate) << "Could not create root isolate.";
88-
89-
// The root isolate ivar is weak.
90-
self->root_isolate_ = strong_root_isolate;
91-
92-
strong_root_isolate->SetReturnCodeCallback([self](uint32_t code) {
93-
if (!self) {
94-
return;
95-
}
96-
97-
self->root_isolate_return_code_ = {true, code};
98-
});
99-
100-
if (auto* platform_configuration =
101-
self->GetPlatformConfigurationIfAvailable()) {
102-
tonic::DartState::Scope scope(strong_root_isolate);
103-
platform_configuration->DidCreateIsolate();
104-
if (!self->FlushRuntimeStateToIsolate()) {
105-
FML_DLOG(ERROR) << "Could not setup initial isolate state.";
106-
}
107-
} else {
108-
FML_DCHECK(false)
109-
<< "RuntimeController created without window binding.";
110-
}
111-
112-
FML_DCHECK(Dart_CurrentIsolate() == nullptr);
113-
114-
self->client_.OnRootIsolateCreated();
115-
return;
116-
});
117-
118-
// We're still trying to create the root isolate as soon as possible here on
119-
// the UI thread although it's deferred a little bit by
120-
// std::async(std::launch::deferred, ...). So the callers of `GetRootIsolate`
121-
// should get a quick return after this UI thread task.
122-
task_runners_.GetUITaskRunner()->PostTask(
123-
[self = weak_factory_.GetWeakPtr()]() {
124-
if (!self) {
125-
return;
126-
}
127-
128-
self->GetRootIsolate();
129-
});
57+
auto strong_root_isolate =
58+
DartIsolate::CreateRootIsolate(
59+
vm_->GetVMData()->GetSettings(), //
60+
isolate_snapshot_, //
61+
task_runners_, //
62+
std::make_unique<PlatformConfiguration>(this), //
63+
snapshot_delegate_, //
64+
io_manager_, //
65+
unref_queue_, //
66+
image_decoder_, //
67+
p_advisory_script_uri, //
68+
p_advisory_script_entrypoint, //
69+
nullptr, //
70+
isolate_create_callback_, //
71+
isolate_shutdown_callback_ //
72+
)
73+
.lock();
74+
75+
FML_CHECK(strong_root_isolate) << "Could not create root isolate.";
76+
77+
// The root isolate ivar is weak.
78+
root_isolate_ = strong_root_isolate;
79+
80+
strong_root_isolate->SetReturnCodeCallback([this](uint32_t code) {
81+
root_isolate_return_code_ = {true, code};
82+
});
83+
84+
if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) {
85+
tonic::DartState::Scope scope(strong_root_isolate);
86+
platform_configuration->DidCreateIsolate();
87+
if (!FlushRuntimeStateToIsolate()) {
88+
FML_DLOG(ERROR) << "Could not setup initial isolate state.";
89+
}
90+
} else {
91+
FML_DCHECK(false) << "RuntimeController created without window binding.";
92+
}
93+
94+
FML_DCHECK(Dart_CurrentIsolate() == nullptr);
13095
}
13196

13297
RuntimeController::~RuntimeController() {
@@ -142,8 +107,8 @@ RuntimeController::~RuntimeController() {
142107
}
143108
}
144109

145-
bool RuntimeController::IsRootIsolateRunning() {
146-
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
110+
bool RuntimeController::IsRootIsolateRunning() const {
111+
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
147112
if (root_isolate) {
148113
return root_isolate->GetPhase() == DartIsolate::Phase::Running;
149114
}
@@ -269,7 +234,7 @@ bool RuntimeController::ReportTimings(std::vector<int64_t> timings) {
269234
}
270235

271236
bool RuntimeController::NotifyIdle(int64_t deadline) {
272-
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
237+
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
273238
if (!root_isolate) {
274239
return false;
275240
}
@@ -326,7 +291,7 @@ bool RuntimeController::DispatchSemanticsAction(int32_t id,
326291

327292
PlatformConfiguration*
328293
RuntimeController::GetPlatformConfigurationIfAvailable() {
329-
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
294+
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
330295
return root_isolate ? root_isolate->platform_configuration() : nullptr;
331296
}
332297

@@ -388,17 +353,17 @@ RuntimeController::ComputePlatformResolvedLocale(
388353
}
389354

390355
Dart_Port RuntimeController::GetMainPort() {
391-
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
356+
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
392357
return root_isolate ? root_isolate->main_port() : ILLEGAL_PORT;
393358
}
394359

395360
std::string RuntimeController::GetIsolateName() {
396-
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
361+
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
397362
return root_isolate ? root_isolate->debug_name() : "";
398363
}
399364

400365
bool RuntimeController::HasLivePorts() {
401-
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
366+
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
402367
if (!root_isolate) {
403368
return false;
404369
}
@@ -407,20 +372,11 @@ bool RuntimeController::HasLivePorts() {
407372
}
408373

409374
tonic::DartErrorHandleType RuntimeController::GetLastError() {
410-
std::shared_ptr<DartIsolate> root_isolate = GetRootIsolate().lock();
375+
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
411376
return root_isolate ? root_isolate->GetLastError() : tonic::kNoError;
412377
}
413378

414379
std::weak_ptr<DartIsolate> RuntimeController::GetRootIsolate() {
415-
std::shared_ptr<DartIsolate> root_isolate = root_isolate_.lock();
416-
if (root_isolate) {
417-
return root_isolate_;
418-
}
419-
420-
// Root isolate is not yet created, get it and do some configuration.
421-
FML_DCHECK(create_and_config_root_isolate_.valid());
422-
create_and_config_root_isolate_.get();
423-
424380
return root_isolate_;
425381
}
426382

‎runtime/runtime_controller.h

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
#ifndef FLUTTER_RUNTIME_RUNTIME_CONTROLLER_H_
66
#define FLUTTER_RUNTIME_RUNTIME_CONTROLLER_H_
77

8-
#include <future>
98
#include <memory>
109
#include <vector>
1110

@@ -341,7 +340,7 @@ class RuntimeController : public PlatformConfigurationClient {
341340
///
342341
/// @return True if root isolate running, False otherwise.
343342
///
344-
virtual bool IsRootIsolateRunning();
343+
virtual bool IsRootIsolateRunning() const;
345344

346345
//----------------------------------------------------------------------------
347346
/// @brief Dispatch the specified platform message to running root
@@ -423,10 +422,7 @@ class RuntimeController : public PlatformConfigurationClient {
423422
/// @brief Get a weak pointer to the root Dart isolate. This isolate may
424423
/// only be locked on the UI task runner. Callers use this
425424
/// accessor to transition to the root isolate to the running
426-
/// phase. Note that it might take times if the isolate is not yet
427-
/// created, which should be done in a subsequence task after
428-
/// constructing `RuntimeController`, or it should get a quick
429-
/// return otherwise.
425+
/// phase.
430426
///
431427
/// @return The root isolate reference.
432428
///
@@ -475,16 +471,11 @@ class RuntimeController : public PlatformConfigurationClient {
475471
std::string advisory_script_entrypoint_;
476472
std::function<void(int64_t)> idle_notification_callback_;
477473
PlatformData platform_data_;
478-
std::future<void> create_and_config_root_isolate_;
479-
// Note that `root_isolate_` is created asynchronously from the constructor of
480-
// `RuntimeController`, be careful to use it directly while it might have not
481-
// been created yet. Call `GetRootIsolate()` instead which guarantees that.
482474
std::weak_ptr<DartIsolate> root_isolate_;
483475
std::pair<bool, uint32_t> root_isolate_return_code_ = {false, 0};
484476
const fml::closure isolate_create_callback_;
485477
const fml::closure isolate_shutdown_callback_;
486478
std::shared_ptr<const fml::Mapping> persistent_isolate_data_;
487-
fml::WeakPtrFactory<RuntimeController> weak_factory_;
488479

489480
PlatformConfiguration* GetPlatformConfigurationIfAvailable();
490481

‎runtime/runtime_delegate.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ class RuntimeDelegate {
3232

3333
virtual FontCollection& GetFontCollection() = 0;
3434

35-
virtual void OnRootIsolateCreated() = 0;
36-
3735
virtual void UpdateIsolateDescription(const std::string isolate_name,
3836
int64_t isolate_port) = 0;
3937

‎shell/common/engine.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -495,10 +495,6 @@ void Engine::HandlePlatformMessage(fml::RefPtr<PlatformMessage> message) {
495495
}
496496
}
497497

498-
void Engine::OnRootIsolateCreated() {
499-
delegate_.OnRootIsolateCreated();
500-
}
501-
502498
void Engine::UpdateIsolateDescription(const std::string isolate_name,
503499
int64_t isolate_port) {
504500
delegate_.UpdateIsolateDescription(isolate_name, isolate_port);

‎shell/common/engine.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -185,15 +185,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
185185
///
186186
virtual void OnPreEngineRestart() = 0;
187187

188-
//--------------------------------------------------------------------------
189-
/// @brief Notifies the shell that the root isolate is created.
190-
/// Currently, this information is to add to the service
191-
/// protocol list of available root isolates running in the VM
192-
/// and their names so that the appropriate isolate can be
193-
/// selected in the tools for debugging and instrumentation.
194-
///
195-
virtual void OnRootIsolateCreated() = 0;
196-
197188
//--------------------------------------------------------------------------
198189
/// @brief Notifies the shell of the name of the root isolate and its
199190
/// port when that isolate is launched, restarted (in the
@@ -821,9 +812,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
821812
// |RuntimeDelegate|
822813
void HandlePlatformMessage(fml::RefPtr<PlatformMessage> message) override;
823814

824-
// |RuntimeDelegate|
825-
void OnRootIsolateCreated() override;
826-
827815
// |RuntimeDelegate|
828816
void UpdateIsolateDescription(const std::string isolate_name,
829817
int64_t isolate_port) override;

‎shell/common/engine_unittests.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ class MockDelegate : public Engine::Delegate {
2424
MOCK_METHOD1(OnEngineHandlePlatformMessage,
2525
void(fml::RefPtr<PlatformMessage>));
2626
MOCK_METHOD0(OnPreEngineRestart, void());
27-
MOCK_METHOD0(OnRootIsolateCreated, void());
2827
MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t));
2928
MOCK_METHOD1(SetNeedsReportTimings, void(bool));
3029
MOCK_METHOD1(ComputePlatformResolvedLocale,
@@ -47,7 +46,6 @@ class MockRuntimeDelegate : public RuntimeDelegate {
4746
void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates));
4847
MOCK_METHOD1(HandlePlatformMessage, void(fml::RefPtr<PlatformMessage>));
4948
MOCK_METHOD0(GetFontCollection, FontCollection&());
50-
MOCK_METHOD0(OnRootIsolateCreated, void());
5149
MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t));
5250
MOCK_METHOD1(SetNeedsReportTimings, void(bool));
5351
MOCK_METHOD1(ComputePlatformResolvedLocale,
@@ -59,7 +57,7 @@ class MockRuntimeController : public RuntimeController {
5957
public:
6058
MockRuntimeController(RuntimeDelegate& client, TaskRunners p_task_runners)
6159
: RuntimeController(client, p_task_runners) {}
62-
MOCK_METHOD0(IsRootIsolateRunning, bool());
60+
MOCK_CONST_METHOD0(IsRootIsolateRunning, bool());
6361
MOCK_METHOD1(DispatchPlatformMessage, bool(fml::RefPtr<PlatformMessage>));
6462
};
6563

‎shell/common/shell.cc

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,8 @@ bool Shell::Setup(std::unique_ptr<PlatformView> platform_view,
562562

563563
is_setup_ = true;
564564

565+
vm_->GetServiceProtocol()->AddHandler(this, GetServiceProtocolDescription());
566+
565567
PersistentCache::GetCacheForProcess()->AddWorkerTaskRunner(
566568
task_runners_.GetIOTaskRunner());
567569

@@ -1131,19 +1133,6 @@ void Shell::OnPreEngineRestart() {
11311133
latch.Wait();
11321134
}
11331135

1134-
// |Engine::Delegate|
1135-
void Shell::OnRootIsolateCreated() {
1136-
auto description = GetServiceProtocolDescription();
1137-
fml::TaskRunner::RunNowOrPostTask(
1138-
task_runners_.GetPlatformTaskRunner(),
1139-
[self = weak_factory_.GetWeakPtr(),
1140-
description = std::move(description)]() {
1141-
if (self) {
1142-
self->vm_->GetServiceProtocol()->AddHandler(self.get(), description);
1143-
}
1144-
});
1145-
}
1146-
11471136
// |Engine::Delegate|
11481137
void Shell::UpdateIsolateDescription(const std::string isolate_name,
11491138
int64_t isolate_port) {
@@ -1288,15 +1277,9 @@ bool Shell::HandleServiceProtocolMessage(
12881277
// |ServiceProtocol::Handler|
12891278
ServiceProtocol::Handler::Description Shell::GetServiceProtocolDescription()
12901279
const {
1291-
FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread());
1292-
1293-
if (!weak_engine_) {
1294-
return ServiceProtocol::Handler::Description();
1295-
}
1296-
12971280
return {
1298-
weak_engine_->GetUIIsolateMainPort(),
1299-
weak_engine_->GetUIIsolateName(),
1281+
engine_->GetUIIsolateMainPort(),
1282+
engine_->GetUIIsolateName(),
13001283
};
13011284
}
13021285

‎shell/common/shell.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -513,9 +513,6 @@ class Shell final : public PlatformView::Delegate,
513513
// |Engine::Delegate|
514514
void OnPreEngineRestart() override;
515515

516-
// |Engine::Delegate|
517-
void OnRootIsolateCreated() override;
518-
519516
// |Engine::Delegate|
520517
void UpdateIsolateDescription(const std::string isolate_name,
521518
int64_t isolate_port) override;

0 commit comments

Comments
 (0)