DEV/flutter_flutter
1
0
Fork 0
mirror of https://github.com/flutter/flutter.git synced 2026-02-20 02:29:02 +08:00
Code Issues Packages Projects Releases Wiki Activity

Do not serialize on UI/Raster threads for Shell::OnCreatePlatformView (flutter/engine#29145)

Browse Source 
* Do not serialize on UI/Raster threads for Shell::OnCreatePlatformView
This commit is contained in:
Dan Field 2021-10-18 14:46:46 -07:00 committed by GitHub
parent e49a357627
commit 59847e64bd
3 changed files with 115 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
engine/src/flutter/shell/common/fixtures/shell_test.dart
View File

@ -13,6 +13,29 @@ void nativeReportTimingsCallback(List<int> timings) native 'NativeReportTimingsC
void nativeOnBeginFrame(int microseconds) native 'NativeOnBeginFrame';
void nativeOnPointerDataPacket(List<int> sequences) native 'NativeOnPointerDataPacket';
@pragma('vm:entry-point')
void drawFrames() {
// Wait for native to tell us to start going.
notifyNative();
PlatformDispatcher.instance.onBeginFrame = (Duration beginTime) {
final SceneBuilder builder = SceneBuilder();
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
canvas.drawPaint(Paint()..color = const Color(0xFFABCDEF));
final Picture picture = recorder.endRecording();
builder.addPicture(Offset.zero, picture);
final Scene scene = builder.build();
window.render(scene);
scene.dispose();
picture.dispose();
};
PlatformDispatcher.instance.scheduleFrame();
}
@pragma('vm:entry-point')
void reportTimingsMain() {
PlatformDispatcher.instance.onReportTimings = (List<FrameTiming> timings) {

90
engine/src/flutter/shell/common/shell.cc
View File

@ -733,16 +733,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.
@ -751,29 +746,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.
@ -786,7 +767,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) {
@ -796,9 +779,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);
@ -826,26 +816,15 @@ void Shell::OnPlatformViewDestroyed() {
// configuration is changed by a task, and the assumption is no longer true.
//
// This incorrect assumption can lead to deadlock.
// 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 platform/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;
@ -855,7 +834,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();
};
@ -870,37 +849,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|

62
engine/src/flutter/shell/common/shell_unittests.cc
View File

@ -3098,5 +3098,67 @@ TEST_F(ShellTest, PrefetchDefaultFontManager) {
DestroyShell(std::move(shell));
}
TEST_F(ShellTest, OnPlatformViewCreatedWhenUIThreadIsBusy) {
// This test will deadlock if the threading logic in
// Shell::OnCreatePlatformView is wrong.
auto settings = CreateSettingsForFixture();
auto shell = CreateShell(std::move(settings));
fml::AutoResetWaitableEvent latch;
fml::TaskRunner::RunNowOrPostTask(shell->GetTaskRunners().GetUITaskRunner(),
[&latch]() { latch.Wait(); });
ShellTest::PlatformViewNotifyCreated(shell.get());
latch.Signal();
DestroyShell(std::move(shell));
}
TEST_F(ShellTest, UIWorkAfterOnPlatformViewDestroyed) {
auto settings = CreateSettingsForFixture();
auto shell = CreateShell(std::move(settings));
auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("drawFrames");
fml::AutoResetWaitableEvent latch;
fml::AutoResetWaitableEvent notify_native_latch;
AddNativeCallback("NotifyNative", CREATE_NATIVE_ENTRY([&](auto args) {
notify_native_latch.Signal();
latch.Wait();
}));
RunEngine(shell.get(), std::move(configuration));
// Wait to make sure we get called back from Dart and thus have latched
// the UI thread before we create/destroy the platform view.
notify_native_latch.Wait();
ShellTest::PlatformViewNotifyCreated(shell.get());
fml::AutoResetWaitableEvent destroy_latch;
fml::TaskRunner::RunNowOrPostTask(
shell->GetTaskRunners().GetPlatformTaskRunner(),
[&shell, &destroy_latch]() {
shell->GetPlatformView()->NotifyDestroyed();
destroy_latch.Signal();
});
destroy_latch.Wait();
// Unlatch the UI thread and let it send us a scene to render.
latch.Signal();
// Flush the UI task runner to make sure we process the render/scheduleFrame
// request.
fml::AutoResetWaitableEvent ui_flush_latch;
fml::TaskRunner::RunNowOrPostTask(shell->GetTaskRunners().GetUITaskRunner(),
[&ui_flush_latch]() {
FML_LOG(ERROR) << "123";
ui_flush_latch.Signal();
});
ui_flush_latch.Wait();
DestroyShell(std::move(shell));
}
} // namespace testing
} // namespace flutter