Unverified Commit 997c5f89 authored by Alex Pott's avatar Alex Pott
Browse files

fix: #3625547 CurrentPathStack keeps every Request it has seen, so...

fix: #3625547 CurrentPathStack keeps every Request it has seen, so long-running processes leak memory

By: siegrist
By: amitgoyal
(cherry picked from commit 396e5b8b)
parent 61b964d2
Loading
Loading
Loading
Loading
Loading
+8 −3
Changes for core/lib/Drupal/Core/Path/CurrentPathStack.php: 8 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -15,9 +15,14 @@
class CurrentPathStack {

  /**
   * Static cache of paths.
   * Static cache of paths, keyed by request.
   *
   * @var \SplObjectStorage
   * Weak, so that a request nothing else references any more is released
   * together with its path. Code that matches many synthetic requests in one
   * process, such as a long-running command resolving URLs, would otherwise
   * keep every one of them.
   *
   * @var \WeakMap<\Symfony\Component\HttpFoundation\Request, string>
   */
  protected $paths;

@@ -36,7 +41,7 @@ class CurrentPathStack {
   */
  public function __construct(RequestStack $request_stack) {
    $this->requestStack = $request_stack;
    $this->paths = new \SplObjectStorage();
    $this->paths = new \WeakMap();
  }

  /**
+56 −0
Changes for core/tests/Drupal/Tests/Core/Path/CurrentPathStackTest.php: 56 added lines, 0 removed lines.
Original line number Diff line number Diff line
<?php

declare(strict_types=1);

namespace Drupal\Tests\Core\Path;

use Drupal\Core\Path\CurrentPathStack;
use Drupal\Tests\UnitTestCase;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\Group;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;

/**
 * Tests Drupal\Core\Path\CurrentPathStack.
 */
#[CoversClass(CurrentPathStack::class)]
#[Group('Path')]
class CurrentPathStackTest extends UnitTestCase {

  /**
   * Tests that a path is kept per request, defaulting to the path info.
   */
  public function testPathIsKeptPerRequest(): void {
    $request_stack = new RequestStack();
    $current = Request::create('/node/1');
    $request_stack->push($current);
    $stack = new CurrentPathStack($request_stack);
    $other = Request::create('/node/2');

    $this->assertSame('/node/1', $stack->getPath());
    $stack->setPath('/system/path', $other);

    $this->assertSame('/system/path', $stack->getPath($other));
    $this->assertSame('/node/1', $stack->getPath($current));
  }

  /**
   * Tests that a request nothing else references is released.
   *
   * Code that matches many synthetic requests against the router in one
   * process, such as re-tracking entity usage from drush, would otherwise keep
   * every one of them for the life of the process.
   */
  public function testRequestNothingElseReferencesIsReleased(): void {
    $stack = new CurrentPathStack(new RequestStack());
    $request = Request::create('/node/1');
    $stack->setPath('/node/1', $request);
    $reference = \WeakReference::create($request);

    unset($request);

    $this->assertNull($reference->get());
  }

}