Skip to content

Commit

Permalink
Do not serialize on UI/Raster threads for Shell::OnCreatePlatformView (…
Browse files Browse the repository at this point in the history 
…flutter#29145)
  • Loading branch information
@yx-mike
yx-mike committed Jan 18, 2022
1 parent 1f7f58d commit 7d03471
Showing 1 changed file with 30 additions and 60 deletions.
90 changes: 30 additions & 60 deletions shell/common/shell.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -728,16 +728,11 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
const bool should_post_raster_task =
!task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread();

// Note:
// This is a synchronous operation because certain platforms depend on
// setup/suspension of all activities that may be interacting with the GPU in
// a synchronous fashion.
fml::AutoResetWaitableEvent latch;
auto raster_task =
fml::MakeCopyable([&waiting_for_first_frame = waiting_for_first_frame_,
rasterizer = rasterizer_->GetWeakPtr(), //
surface = std::move(surface), //
&latch]() mutable {
surface = std::move(surface)]() mutable {
if (rasterizer) {
// Enables the thread merger which may be used by the external view
// embedder.
Expand All @@ -746,29 +741,15 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
}

waiting_for_first_frame.store(true);

// Step 3: All done. Signal the latch that the platform thread is
// waiting on.
latch.Signal();
});

auto ui_task = [engine = engine_->GetWeakPtr(), //
raster_task_runner = task_runners_.GetRasterTaskRunner(), //
raster_task, should_post_raster_task,
&latch //
] {
// TODO(91717): This probably isn't necessary. The engine should be able to
// handle things here via normal lifecycle messages.
// https://github.com/flutter/flutter/issues/91717
auto ui_task = [engine = engine_->GetWeakPtr()] {
if (engine) {
engine->OnOutputSurfaceCreated();
}
// Step 2: Next, tell the raster thread that it should create a surface for
// its rasterizer.
if (should_post_raster_task) {
fml::TaskRunner::RunNowOrPostTask(raster_task_runner, raster_task);
} else {
// See comment on should_post_raster_task, in this case we just unblock
// the platform thread.
latch.Signal();
}
};

// Threading: Capture platform view by raw pointer and not the weak pointer.
Expand All @@ -781,7 +762,9 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {

auto io_task = [io_manager = io_manager_->GetWeakPtr(), platform_view,
ui_task_runner = task_runners_.GetUITaskRunner(), ui_task,
shared_resource_context = shared_resource_context_] {
shared_resource_context = shared_resource_context_,
raster_task_runner = task_runners_.GetRasterTaskRunner(),
raster_task, should_post_raster_task, &latch] {
if (io_manager && !io_manager->GetResourceContext()) {
sk_sp<GrDirectContext> resource_context;
if (shared_resource_context) {
Expand All @@ -791,9 +774,16 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
}
io_manager->NotifyResourceContextAvailable(resource_context);
}
// Step 1: Next, post a task on the UI thread to tell the engine that it has
// Step 1: Post a task on the UI thread to tell the engine that it has
// an output surface.
fml::TaskRunner::RunNowOrPostTask(ui_task_runner, ui_task);

// Step 2: Tell the raster thread that it should create a surface for
// its rasterizer.
if (should_post_raster_task) {
fml::TaskRunner::RunNowOrPostTask(raster_task_runner, raster_task);
}
latch.Signal();
};

fml::TaskRunner::RunNowOrPostTask(task_runners_.GetIOTaskRunner(), io_task);
Expand Down Expand Up @@ -821,26 +811,15 @@ void Shell::OnPlatformViewDestroyed() {
// configuration is changed by a task, and the asumption is not longer true.
//
// This incorrect assumption can lead to dead lock.
// See `should_post_raster_task` for more.
rasterizer_->DisableThreadMergerIfNeeded();

// The normal flow executed by this method is that the platform thread is
// starting the sequence and waiting on the latch. Later the UI thread posts
// raster_task to the raster thread triggers signaling the latch(on the IO
// thread). If the raster and the platform threads are the same this results
// in a deadlock as the raster_task will never be posted to the plaform/raster
// thread that is blocked on a latch. To avoid the described deadlock, if the
// raster and the platform threads are the same, should_post_raster_task will
// be false, and then instead of posting a task to the raster thread, the ui
// thread just signals the latch and the platform/raster thread follows with
// executing raster_task.
const bool should_post_raster_task =
!task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread();

// Note:
// This is a synchronous operation because certain platforms depend on
// setup/suspension of all activities that may be interacting with the GPU in
// a synchronous fashion.
// The UI thread does not need to be serialized here - there is sufficient
// guardrailing in the rasterizer to allow the UI thread to post work to it
// even after the surface has been torn down.

fml::AutoResetWaitableEvent latch;

Expand All @@ -850,7 +829,7 @@ void Shell::OnPlatformViewDestroyed() {
io_manager->GetIsGpuDisabledSyncSwitch()->Execute(
fml::SyncSwitch::Handlers().SetIfFalse(
[&] { io_manager->GetSkiaUnrefQueue()->Drain(); }));
// Step 3: All done. Signal the latch that the platform thread is waiting
// Step 4: All done. Signal the latch that the platform thread is waiting
// on.
latch.Signal();
};
Expand All @@ -865,37 +844,28 @@ void Shell::OnPlatformViewDestroyed() {
rasterizer->EnableThreadMergerIfNeeded();
rasterizer->Teardown();
}
// Step 2: Next, tell the IO thread to complete its remaining work.
// Step 3: Tell the IO thread to complete its remaining work.
fml::TaskRunner::RunNowOrPostTask(io_task_runner, io_task);
};

auto ui_task = [engine = engine_->GetWeakPtr(),
raster_task_runner = task_runners_.GetRasterTaskRunner(),
raster_task, should_post_raster_task, &latch]() {
// TODO(91717): This probably isn't necessary. The engine should be able to
// handle things here via normal lifecycle messages.
// https://github.com/flutter/flutter/issues/91717
auto ui_task = [engine = engine_->GetWeakPtr()]() {
if (engine) {
engine->OnOutputSurfaceDestroyed();
}
if (should_post_raster_task) {
fml::TaskRunner::RunNowOrPostTask(raster_task_runner, raster_task);
} else {
// See comment on should_post_raster_task, in this case we just unblock
// the platform thread.
latch.Signal();
}
};

// Step 0: Post a task onto the UI thread to tell the engine that its output
// Step 1: Post a task onto the UI thread to tell the engine that its output
// surface is about to go away.
fml::TaskRunner::RunNowOrPostTask(task_runners_.GetUITaskRunner(), ui_task);

// Step 2: Post a task to the Raster thread (possibly this thread) to tell the
// rasterizer the output surface is going away.
fml::TaskRunner::RunNowOrPostTask(task_runners_.GetRasterTaskRunner(),
raster_task);
latch.Wait();
if (!should_post_raster_task) {
// See comment on should_post_raster_task, in this case the raster_task
// wasn't executed, and we just run it here as the platform thread
// is the raster thread.
raster_task();
latch.Wait();
}
}

// |PlatformView::Delegate|
Expand Down

0 comments on commit 7d03471

Please sign in to comment.