Commit 78f170a9 authored by catch's avatar catch
Browse files

fix: #3553342 Race condition in LocalTaskManager::getLocalTasks() with fibers

By: berdir
By: kristiaanvandeneynde
By: catch
(cherry picked from commit 8adf8b75)
parent 72eb261d
Loading
Loading
Loading
Loading
Loading
+34 −6
Original line number Diff line number Diff line
@@ -106,6 +106,11 @@ class LocalTaskManager extends DefaultPluginManager implements LocalTaskManagerI
   */
  protected $account;

  /**
   * Flag that indicates if local tasks are currently being loaded.
   */
  protected bool $loadingLocalTasks = FALSE;

  /**
   * Constructs a \Drupal\Core\Menu\LocalTaskManager object.
   *
@@ -344,15 +349,26 @@ public function getTasksBuild($current_route_name, RefinableCacheableDependencyI
   * {@inheritdoc}
   */
  public function getLocalTasks($route_name, $level = 0) {

    if ($this->loadingLocalTasks && \Fiber::getCurrent() !== NULL) {
      // Primary and secondary task are rendered in separate blocks, each within
      // their own fiber. Both call this method for a different level, but the
      // data is built for both levels on the first call. If the first call
      // gets suspended, for example due to an entity load in a URL access
      // check, the second block will then call into this. If the data is
      // already being built, and we're in a fiber, suspend once to allow the
      // first fiber to complete building the data. If it is still not done,
      // proceed anyway, which may build that information twice but will not
      // return incomplete local task data.
      \Fiber::suspend();
    }

    if (!isset($this->taskData[$route_name])) {
      $this->loadingLocalTasks = TRUE;
      $cacheability = new CacheableMetadata();
      $cacheability->addCacheContexts(['route']);
      // Look for route-based tabs.
      $this->taskData[$route_name] = [
        'tabs' => [],
        'cacheability' => $cacheability,
      ];

      // Look for route-based tabs.
      if (!$this->requestStack->getCurrentRequest()->attributes->has('exception')) {
        // Safe to build tasks only when no exceptions raised.
        $data = [];
@@ -360,11 +376,23 @@ public function getLocalTasks($route_name, $level = 0) {
        foreach ($local_tasks as $tab_level => $items) {
          $data[$tab_level] = empty($data[$tab_level]) ? $items : array_merge($data[$tab_level], $items);
        }
        $this->taskData[$route_name]['tabs'] = $data;
        $this->taskData[$route_name] = [
          'tabs' => $data,
          'cacheability' => $cacheability,
        ];

        // Allow modules to alter local tasks.
        $this->moduleHandler->alter('menu_local_tasks', $this->taskData[$route_name], $route_name, $cacheability);
        $this->taskData[$route_name]['cacheability'] = $cacheability;
      }
      else {
        $this->taskData[$route_name] = [
          'tabs' => [],
          'cacheability' => $cacheability,
        ];
      }

      $this->loadingLocalTasks = FALSE;
    }

    if (isset($this->taskData[$route_name]['tabs'][$level])) {
+87 −0
Original line number Diff line number Diff line
@@ -439,6 +439,93 @@ public function testGetTasksBuildWithCacheabilityMetadata(): void {
    $this->assertEqualsCanonicalizing(['context.example1', 'context.example2', 'route', 'user.permissions'], $cacheability->getCacheContexts());
  }

  /**
   * Test multiple parallel calls with fibers.
   */
  public function testGetTasksBuildWithFibers(): void {
    $definitions = $this->getLocalTaskFixtures();

    $this->pluginDiscovery->expects($this->once())
      ->method('getDefinitions')
      ->willReturn($definitions);

    $active_plugin_id = 'menu_local_task_test_tasks_view';
    $map = [];

    foreach ($definitions as $plugin_id => $info) {
      $mock = $this->prophesize(LocalTaskInterface::class);
      $mock->willImplement(CacheableDependencyInterface::class);
      $mock->getRouteName()->willReturn($info['route_name']);
      $mock->getTitle()->willReturn($info['title']);
      $mock->getRouteParameters(Argument::cetera())->willReturn([]);
      $mock->getOptions(Argument::cetera())->willReturn([]);
      $mock->getActive()->willReturn($plugin_id === $active_plugin_id);
      $mock->getWeight()->willReturn($info['weight'] ?? 0);
      $mock->getCacheContexts()->willReturn([]);
      $mock->getCacheTags()->willReturn([]);
      $mock->getCacheMaxAge()->willReturn(Cache::PERMANENT);
      $map[] = [$info['id'], [], $mock->reveal()];
    }

    // Simulate an access callback that suspends a fiber.
    $this->accessManager->expects($this->any())
      ->method('checkNamedRoute')
      ->willReturnCallback(function (string $route_name) {
        if ($route_name === 'menu_local_task_test_tasks_edit') {
          \Fiber::suspend();
        }
        return AccessResult::allowed();
      });

    $this->factory->expects($this->any())
      ->method('createInstance')
      ->willReturnMap($map);
    $this->setupLocalTaskManager();

    $this->argumentResolver->expects($this->any())
      ->method('getArguments')
      ->willReturn([]);

    $this->routeMatch->expects($this->any())
      ->method('getRouteName')
      ->willReturn('menu_local_task_test_tasks_view');
    $this->routeMatch->expects($this->any())
      ->method('getRawParameters')
      ->willReturn(new InputBag());

    $first_fiber = new \Fiber(fn () => $this->manager->getLocalTasks('menu_local_task_test_tasks_view', 0));
    $second_fiber = new \Fiber(fn () => $this->manager->getLocalTasks('menu_local_task_test_tasks_view', 1));

    $fibers = [$first_fiber, $second_fiber];
    $suspended = FALSE;
    do {
      foreach ($fibers as $key => $fiber) {
        if (!$fiber->isStarted()) {
          $fiber->start();
        }
        elseif ($fiber->isSuspended()) {
          $suspended = TRUE;
          $fiber->resume();
        }
        elseif ($fiber->isTerminated()) {
          unset($fibers[$key]);
        }
      }
    } while (!empty($fibers));

    // Ensure that the fibers were suspended at least once to make sure that
    // the expected scenario is tested here.
    $this->assertTrue($suspended);

    // Assert that both fibers return the correct result.
    $this->assertEquals([
      'menu_local_task_test_tasks_settings',
      'menu_local_task_test_tasks_edit',
      'menu_local_task_test_tasks_view.tab',
    ], array_keys($first_fiber->getReturn()['tabs']));
    $this->assertEquals(['menu_local_task_test_tasks_view_child1', 'menu_local_task_test_tasks_view_child2'], array_keys($second_fiber->getReturn()['tabs']));
  }

  protected function setupFactoryAndLocalTaskPlugins(array $definitions, $active_plugin_id): void {
    $map = [];
    $access_manager_map = [];