diff --git a/core/lib/Drupal/Component/PhpStorage/FileReadOnlyStorage.php b/core/lib/Drupal/Component/PhpStorage/FileReadOnlyStorage.php index 1a460ad..0325f4f 100644 --- a/core/lib/Drupal/Component/PhpStorage/FileReadOnlyStorage.php +++ b/core/lib/Drupal/Component/PhpStorage/FileReadOnlyStorage.php @@ -34,13 +34,6 @@ public function __construct(array $configuration) { } /** - * Implements Drupal\Component\PhpStorage\PhpStorageInterface::exists(). - */ - public function exists($name) { - return file_exists($this->getFullPath($name)); - } - - /** * Implements Drupal\Component\PhpStorage\PhpStorageInterface::load(). */ public function load($name) { @@ -50,6 +43,13 @@ public function load($name) { } /** + * {@inheritdoc} + */ + public function loadClass($name, $class_name, $wait = 200000, $retry = 5) { + return class_exists($class_name, FALSE) || $this->load($name); + } + + /** * Implements Drupal\Component\PhpStorage\PhpStorageInterface::save(). */ public function save($name, $code) { diff --git a/core/lib/Drupal/Component/PhpStorage/FileStorage.php b/core/lib/Drupal/Component/PhpStorage/FileStorage.php index 71ff3f2..6efc78a 100644 --- a/core/lib/Drupal/Component/PhpStorage/FileStorage.php +++ b/core/lib/Drupal/Component/PhpStorage/FileStorage.php @@ -33,14 +33,33 @@ public function __construct(array $configuration) { } /** - * Implements Drupal\Component\PhpStorage\PhpStorageInterface::exists(). + * {@inheritdoc} */ - public function exists($name) { - return file_exists($this->getFullPath($name)); + public function loadClass($name, $class_name = NULL, $wait = 200000, $retry = 5) { + // If the class does not exist after loading the file a race condition is + // very likely so wait some random time allowing the racing process to + // finish. + while (!($exists = class_exists($class_name, FALSE)) && $this->doLoadClass($name, TRUE) && !($exists = class_exists($class_name, FALSE)) && $retry--) { + usleep(mt_rand($wait, 2 * $wait)); + } + return $exists; } /** - * Implements Drupal\Component\PhpStorage\PhpStorageInterface::load(). + * Do the actual class loading. + * + * @param $name + * The name of the file. + * + * @return bool + * TRUE on success, FALSE otherwise. + */ + protected function doLoadClass($name) { + return (@include $this->getFullPath($name)) !== FALSE; + } + + /** + * {@inheritdoc} */ public function load($name) { // The FALSE returned on failure is enough for the caller to handle this, diff --git a/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFileStorage.php b/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFileStorage.php index 573e23c..da593b8 100644 --- a/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFileStorage.php +++ b/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFileStorage.php @@ -47,10 +47,14 @@ public function load($name) { } /** - * Implements Drupal\Component\PhpStorage\PhpStorageInterface::exists(). + * {@inheritdoc} */ - public function exists($name) { - return $this->checkFile($name) !== FALSE; + protected function doLoadClass($name) { + if (($filename = $this->checkFile($name)) !== FALSE) { + // Inline parent::load() to avoid an expensive getFullPath() call. + return (@include $filename) !== FALSE; + } + return FALSE; } /** diff --git a/core/lib/Drupal/Component/PhpStorage/PhpStorageInterface.php b/core/lib/Drupal/Component/PhpStorage/PhpStorageInterface.php index 1f6a682..a2da2cd 100644 --- a/core/lib/Drupal/Component/PhpStorage/PhpStorageInterface.php +++ b/core/lib/Drupal/Component/PhpStorage/PhpStorageInterface.php @@ -19,31 +19,48 @@ interface PhpStorageInterface { /** - * Checks whether the PHP code exists in storage. - * - * @param string $name - * The virtual file name. Can be a relative path. - * - * @return bool - * TRUE if the virtual file exists, FALSE otherwise. - */ - public function exists($name); - - /** * Loads PHP code from storage. * * Depending on storage implementation, exists() checks can be expensive, so * this function may be called for a file that doesn't exist, and that should - * not result in errors. This function does not return anything, so it is - * up to the caller to determine if any code was loaded (for example, check - * class_exists() or function_exists() for what was expected in the code). + * not result in errors. * * @param string $name * The virtual file name. Can be a relative path. + * + * @return + * TRUE on success, FALSE otherwise. This only means a file was loaded, the + * contents were not checked, that's up to the caller. Check the + * loadClass() method for loading classes. */ public function load($name); /** + * Loads a class from storage. + * + * Depending on storage implementation, exists() checks can be expensive, so + * this function may be called for a file that doesn't exist, and that should + * not result in errors. + * + * @param string $name + * The virtual file name. Can be a relative path. + * @param string $class_name + * A class name to verify after loading. In a race condition, the file + * might exist but not contain the right value. + * @param int $wait + * If the file is found but the class does not exist then a race condition + * is very likely so wait a random amount of time between $wait and + * 2 * $wait microseconds before returning to allow the racing process to + * finish. + * @param int $retry + * The number of times to retry waiting and reloading. + * + * @return + * TRUE on success, FALSE otherwise. + */ + public function loadClass($name, $class_name, $wait = 200000, $retry = 5); + + /** * Saves PHP code to storage. * * @param string $name diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index 557dfd1..6fcecbb 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -720,11 +720,12 @@ protected function initializeContainer($rebuild = FALSE) { // If the module list hasn't already been set in updateModules and we are // not forcing a rebuild, then try and load the container from the disk. if (empty($this->moduleList) && !$rebuild) { - $fully_qualified_class_name = '\\' . $this->getClassNamespace() . '\\' . $this->getClassName(); + $class_name = $this->getClassName(); + $fully_qualified_class_name = '\\' . $this->getClassNamespace() . '\\' . $class_name; // First, try to load from storage. if (!class_exists($fully_qualified_class_name, FALSE)) { - $this->storage()->load($this->getClassName() . '.php'); + $this->storage()->load($class_name . '.php', $fully_qualified_class_name); } // If the load succeeded or the class already existed, use it. if (class_exists($fully_qualified_class_name, FALSE)) { diff --git a/core/lib/Drupal/Core/Template/TwigEnvironment.php b/core/lib/Drupal/Core/Template/TwigEnvironment.php index 9225712..3807888 100644 --- a/core/lib/Drupal/Core/Template/TwigEnvironment.php +++ b/core/lib/Drupal/Core/Template/TwigEnvironment.php @@ -127,9 +127,9 @@ public function loadTemplate($name, $index = NULL) { $this->updateCompiledTemplate($cache_filename, $name); } - if (!$this->storage()->load($cache_filename)) { + if (!$this->storage()->loadClass($cache_filename, $cls)) { $this->updateCompiledTemplate($cache_filename, $name); - $this->storage()->load($cache_filename); + $this->storage()->loadClass($cache_filename, $cls); } } } diff --git a/core/modules/system/src/Tests/Theme/TwigSettingsTest.php b/core/modules/system/src/Tests/Theme/TwigSettingsTest.php index 36cc48d..0ab9773 100644 --- a/core/modules/system/src/Tests/Theme/TwigSettingsTest.php +++ b/core/modules/system/src/Tests/Theme/TwigSettingsTest.php @@ -105,7 +105,7 @@ function testTwigCacheOverride() { // Navigate to the page and make sure the template gets cached. $this->drupalGet('theme-test/template-test'); - $this->assertTrue(PhpStorageFactory::get('twig')->exists($cache_filename), 'Cached Twig template found.'); + $this->assertTrue(PhpStorageFactory::get('twig')->load($cache_filename), 'Cached Twig template found.'); // Disable the Twig cache and rebuild the service container. $parameters = $this->container->getParameter('twig.config'); diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php b/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php index 8cf9642..2516465 100644 --- a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php +++ b/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageReadOnlyTest.php @@ -71,7 +71,7 @@ public function testReadOnly() { // If the file was successfully loaded, it must also exist, but ensure the // exists() method returns that correctly. - $this->assertSame($php_read->exists($name), TRUE); + $this->assertSame($php_read->load($name), TRUE); // Saving and deleting should always fail. $this->assertFalse($php_read->save($name, $code)); $this->assertFalse($php_read->delete($name)); diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php b/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php index e0a7d81..024b566 100644 --- a/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php +++ b/core/tests/Drupal/Tests/Component/PhpStorage/FileStorageTest.php @@ -40,7 +40,6 @@ protected function setUp() { * * @covers ::load * @covers ::save - * @covers ::exists * @covers ::delete */ public function testCRUD() { diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageBase.php b/core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageBase.php index 5667e42..86041e1 100644 --- a/core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageBase.php +++ b/core/tests/Drupal/Tests/Component/PhpStorage/MTimeProtectedFileStorageBase.php @@ -54,7 +54,6 @@ protected function setUp() { * @covers ::load * @covers ::save * @covers ::delete - * @covers ::exists */ public function testCRUD() { $php = new $this->storageClass($this->settings); @@ -71,6 +70,7 @@ public function testCRUD() { * @medium */ public function testSecurity() { + /** @var \Drupal\Component\PhpStorage\PhpStorageInterface $php */ $php = new $this->storageClass($this->settings); $name = 'simpletest.php'; $php->save($name, 'assertSame(file_get_contents($expected_filename), $untrusted_code); - $this->assertSame($php->exists($name), $this->expected[$i]); $this->assertSame($php->load($name), $this->expected[$i]); $this->assertSame($GLOBALS['hacked'], $this->expected[$i]); } diff --git a/core/tests/Drupal/Tests/Component/PhpStorage/PhpStorageTestBase.php b/core/tests/Drupal/Tests/Component/PhpStorage/PhpStorageTestBase.php index fabf301..535c272 100644 --- a/core/tests/Drupal/Tests/Component/PhpStorage/PhpStorageTestBase.php +++ b/core/tests/Drupal/Tests/Component/PhpStorage/PhpStorageTestBase.php @@ -46,16 +46,12 @@ public function assertCRUD($php) { $code = "save($name, $code); $this->assertTrue($success, 'Saved php file'); - $php->load($name); + $this->assertTrue($php->load($name), 'Load works correctly'); $this->assertTrue($GLOBALS[$random], 'File saved correctly with correct value'); - // If the file was successfully loaded, it must also exist, but ensure the - // exists() method returns that correctly. - $this->assertTrue($php->exists($name), 'Exists works correctly'); - // Delete the file, and then ensure exists() returns FALSE. $this->assertTrue($php->delete($name), 'Delete suceeded'); - $this->assertFalse($php->exists($name), 'Delete deleted file'); + $this->assertFalse($php->load($name), 'Delete deleted file'); // Ensure delete() can be called on a non-existing file. It should return // FALSE, but not trigger errors.