Avoid deadlock while creating platform views with multiple engines.

[eggfly]

When using multiple lightweight engines and multiple platform views, it sometimes will result in a deadlock.

The code in shell.cc:

void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
  TRACE_EVENT0("flutter", "Shell::OnPlatformViewCreated");
  FML_DCHECK(is_setup_);
  FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread());

  // Prevent any request to change the thread configuration for raster and
  // platform queues while the platform view is being created.
  //
  // This prevents false positives such as this method starts assuming that the
  // raster and platform queues have a given thread configuration, but then the
  // 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 which signals the latch. 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 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.
  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 {
        if (rasterizer) {
          // Enables the thread merger which may be used by the external view
          // embedder.
          rasterizer->EnableThreadMergerIfNeeded();
          rasterizer->Setup(std::move(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  //
  ] {
    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.
  // We are going to use the pointer on the IO thread which is not safe with a
  // weak pointer. However, we are preventing the platform view from being
  // collected by using a latch.
  auto* platform_view = platform_view_.get();

  FML_DCHECK(platform_view);

  auto io_task = [io_manager = io_manager_->GetWeakPtr(), platform_view,
                  ui_task_runner = task_runners_.GetUITaskRunner(), ui_task,
                  shared_resource_context = shared_resource_context_] {
    if (io_manager && !io_manager->GetResourceContext()) {
      sk_sp<GrDirectContext> resource_context;
      if (shared_resource_context) {
        resource_context = shared_resource_context;
      } else {
        resource_context = platform_view->CreateResourceContext();
      }
      io_manager->NotifyResourceContextAvailable(resource_context);
    }
    // Step 1: Next, 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);
  };

  fml::TaskRunner::RunNowOrPostTask(task_runners_.GetIOTaskRunner(), io_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();
  }
}

The variable should_post_raster_task in OnPlatformViewCreated() controls the raster_task should be post to platform/raster thread or runs directly.

Before this change, the method DisableThreadMergerIfNeeded() will set the enabled state into RasterThreadMerger instance and not shared when two same thread pair (lightweight engines use same raster and platform thread). So this change move the enabled_ member from RasterThreadMerger class to SharedThreadMerger class.

It was my little negligence not move this value into the SharedThreadMerger class. 😂

Pre-launch Checklist

• I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
• I read the [Tree Hygiene] wiki page, which explains my responsibilities.
• I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the [CLA].
• All existing and new tests are passing.

[eggfly]

@iskakaushik @gaaclarke Please have a review, thx~

[iskakaushik]

LGTM

[gaaclarke]

It's a bit odd that the writing of this is in a mutex lock, where reading is not.

[eggfly]

@gaaclarke Thanks! 💪 I also have some changes about the thread merger, I will deal with it together with another PR.

[gaaclarke]

I'm a bit uncertain about which threads are allowed to call this.

[eggfly]

OK, I will think about it