diff --git a/core/lib/Drupal/Core/Routing/RouteBuilder.php b/core/lib/Drupal/Core/Routing/RouteBuilder.php index a4b17e2..e235e39 100644 --- a/core/lib/Drupal/Core/Routing/RouteBuilder.php +++ b/core/lib/Drupal/Core/Routing/RouteBuilder.php @@ -133,6 +133,16 @@ public function rebuild() { // We choose to block here since otherwise the routes might not be // available, resulting in a 404. $this->lock->wait('router_rebuild'); + + if ($this->routeBuilderIndicator->isRebuildStillNeeded()) { + // Per default the lock waits for 30 seconds. In case we haven't + // completed in the meantime, try to rebuild again. In case the + // rebuilding needs an incredible long time, and the callstack by this + // recursive call gets high, you already have a problem with your + // site being down for a long long time. + $this->rebuild(); + } + return FALSE; } @@ -215,6 +225,15 @@ public function getCollectionDuringRebuild() { */ public function rebuildIfNeeded() { if ($this->routeBuilderIndicator->isRebuildNeeded()) { + // In case we need to rebuild the router, absolutely ensure that we need + // to rebuild the router. There is the small chance (which gets high in + // case there are a lot of requests to the page), that we have actually + // completed the rebuild in the meantime, but the internal indicator + // (for example state) is still static cached in this process. Therefore + // use an safe, but slow way, to check whether the rebuild is needed. + if (!$this->routeBuilderIndicator->isRebuildStillNeeded()) { + return FALSE; + } return $this->rebuild(); } return FALSE; diff --git a/core/lib/Drupal/Core/Routing/RouteBuilderIndicator.php b/core/lib/Drupal/Core/Routing/RouteBuilderIndicator.php index 5f06120..da99bf2 100644 --- a/core/lib/Drupal/Core/Routing/RouteBuilderIndicator.php +++ b/core/lib/Drupal/Core/Routing/RouteBuilderIndicator.php @@ -52,4 +52,14 @@ public function setRebuildDone() { $this->state->set(static::REBUILD_NEEDED, FALSE); } + /** + * {@inheritdoc} + */ + public function isRebuildStillNeeded() { + // To absolutely ensure that the router rebuild is required, reset the cache + // in case they were set by another process. + $this->state->resetCache(); + return $this->isRebuildNeeded(); + } + } diff --git a/core/lib/Drupal/Core/Routing/RouteBuilderIndicatorInterface.php b/core/lib/Drupal/Core/Routing/RouteBuilderIndicatorInterface.php index 68ce5b8..fcd8495 100644 --- a/core/lib/Drupal/Core/Routing/RouteBuilderIndicatorInterface.php +++ b/core/lib/Drupal/Core/Routing/RouteBuilderIndicatorInterface.php @@ -36,4 +36,16 @@ public function setRebuildDone(); */ public function isRebuildNeeded(); + /** + * Checks if the router rebuild really needed. + * + * This method should not rely on any static caching. + * + * This method is called during route rebuilding if acquiring a lock failed. + * + * @return bool + * TRUE if the router still needs to be rebuilt, FALSE if not. + */ + public function isRebuildStillNeeded(); + } diff --git a/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php b/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php index b0efe77..d25cc58 100644 --- a/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php @@ -278,9 +278,13 @@ public function testRebuildIfNecessary() { $this->routeBuilderIndicator->expects($this->once()) ->method('setRebuildDone'); - $this->routeBuilderIndicator->expects($this->exactly(2)) + $this->routeBuilderIndicator->expects($this->exactly(3)) ->method('isRebuildNeeded') - ->will($this->onConsecutiveCalls(TRUE, FALSE)); + ->will($this->onConsecutiveCalls(TRUE, TRUE, FALSE)); + + $this->routeBuilderIndicator->expects($this->exactly(2)) + ->method('isRebuildStillNeeded') + ->will($this->onConsecutiveCalls(TRUE, FALSE)); $this->yamlDiscovery->expects($this->any()) ->method('findAll') @@ -293,6 +297,9 @@ public function testRebuildIfNecessary() { // This will not trigger a rebuild. $this->assertFalse($this->routeBuilder->rebuildIfNeeded()); + + // This will not trigger a rebuild, because the indicator is not set at all. + $this->assertFalse($this->routeBuilder->rebuildIfNeeded()); } }