diff --git a/core/lib/Drupal/Component/PhpStorage/FileReadOnlyStorage.php b/core/lib/Drupal/Component/PhpStorage/FileReadOnlyStorage.php index 1a460ad..278a823 100644 --- a/core/lib/Drupal/Component/PhpStorage/FileReadOnlyStorage.php +++ b/core/lib/Drupal/Component/PhpStorage/FileReadOnlyStorage.php @@ -34,19 +34,21 @@ public function __construct(array $configuration) { } /** - * Implements Drupal\Component\PhpStorage\PhpStorageInterface::exists(). + * {@inheritdoc} */ - public function exists($name) { - return file_exists($this->getFullPath($name)); + public function load($name, callable $success = NULL, $force = FALSE, $wait = 200000, $retry = 5) { + // The FALSE returned on failure is enough for the caller to handle this, + // we do not want a warning too. Also, no race conditions in read only + // storage so ignore all those parameters. + return (@include_once $this->getFullPath($name)) !== FALSE; } /** - * Implements Drupal\Component\PhpStorage\PhpStorageInterface::load(). + * {@inheritdoc} */ - public function load($name) { - // The FALSE returned on failure is enough for the caller to handle this, - // we do not want a warning too. - return (@include_once $this->getFullPath($name)) !== FALSE; + public function loadClass($name, $class_name, $force = FALSE, $wait = 200000, $retry = 5) { + // No race conditions in read only storage, huzzah! + return $this->load($name); } /** diff --git a/core/lib/Drupal/Component/PhpStorage/FileStorage.php b/core/lib/Drupal/Component/PhpStorage/FileStorage.php index 71ff3f2..bef7a05 100644 --- a/core/lib/Drupal/Component/PhpStorage/FileStorage.php +++ b/core/lib/Drupal/Component/PhpStorage/FileStorage.php @@ -33,18 +33,60 @@ public function __construct(array $configuration) { } /** - * Implements Drupal\Component\PhpStorage\PhpStorageInterface::exists(). + * {@inheritdoc} */ - public function exists($name) { - return file_exists($this->getFullPath($name)); + public function load($name, callable $success = NULL, $force = FALSE, $wait = 200000, $retry = 5) { + if (!isset($success)) { + // Without a success callback, the only way to be safe is to include the + // file once. + return $force ? $this->doInclude($name) : $this->doIncludeOnce($name); + } + if (!$force && $success()) { + return TRUE; + } + // If the success callback fails after loading the file a race condition is + // very likely so wait some random time allowing the racing process to + // finish. + while ($this->doInclude($name) && !($return = $success()) && $retry--) { + usleep(mt_rand($wait, 2 * $wait)); + } + // If the include failed then return is not set but then returning FALSE + // is correct. + return !empty($return); } /** - * Implements Drupal\Component\PhpStorage\PhpStorageInterface::load(). + * {@inheritdoc} */ - public function load($name) { + public function loadClass($name, $class_name = NULL, $force = FALSE, $wait = 200000, $retry = 5) { // The FALSE returned on failure is enough for the caller to handle this, // we do not want a warning too. + return $this->load($name, function () use($class_name) { return class_exists($class_name, FALSE); }, $wait, $retry); + } + + /** + * Run the actual include. + * + * @param $name + * The name of the file. + * + * @return bool + * TRUE on success, FALSE otherwise. + */ + protected function doInclude($name) { + return (@include $this->getFullPath($name)) !== FALSE; + } + + /** + * Run the actual include_once. + * + * @param $name + * The name of the file. + * + * @return bool + * TRUE on success, FALSE otherwise. + */ + protected function doIncludeOnce($name) { return (@include_once $this->getFullPath($name)) !== FALSE; } diff --git a/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFileStorage.php b/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFileStorage.php index 573e23c..ba0f66f 100644 --- a/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFileStorage.php +++ b/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFileStorage.php @@ -36,21 +36,25 @@ class MTimeProtectedFileStorage extends MTimeProtectedFastFileStorage { /** - * Implements Drupal\Component\PhpStorage\PhpStorageInterface::load(). + * {@inheritdoc} */ - public function load($name) { + protected function doInclude($name) { if (($filename = $this->checkFile($name)) !== FALSE) { - // Inline parent::load() to avoid an expensive getFullPath() call. - return (@include_once $filename) !== FALSE; + // Inline parent::doInclude() to avoid an expensive getFullPath() call. + return (@include $filename) !== FALSE; } return FALSE; } /** - * Implements Drupal\Component\PhpStorage\PhpStorageInterface::exists(). + * {@inheritdoc} */ - public function exists($name) { - return $this->checkFile($name) !== FALSE; + protected function doIncludeOnce($name) { + if (($filename = $this->checkFile($name)) !== FALSE) { + // Inline parent::doIncludeOnce() to avoid an expensive getFullPath() call. + return (@include_once $filename) !== FALSE; + } + return FALSE; } /** diff --git a/core/lib/Drupal/Component/PhpStorage/PhpStorageInterface.php b/core/lib/Drupal/Component/PhpStorage/PhpStorageInterface.php index 1f6a682..587c56d 100644 --- a/core/lib/Drupal/Component/PhpStorage/PhpStorageInterface.php +++ b/core/lib/Drupal/Component/PhpStorage/PhpStorageInterface.php @@ -19,29 +19,65 @@ 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). + * This function may be called for a file that doesn't exist, and that should + * not result in errors. + * + * It is safe to call this function multiple times: the relevant code will + * only be loaded once. * * @param string $name * The virtual file name. Can be a relative path. + * @param callable $success + * A race condition might cause the file to exist but be empty. If this + * parameter is specified the system will retry loading if the callback + * return FALSE to allow the racing process to finish. It is strongly + * recommended to supply this argument or use loadClass() instead to load + * a class. Otherwise, if the same file is written by anotehr process, + * a false positive might occur. + * @param bool $force + * Override the safety guarantee and force loading. For example, the caller + * might've already determined the code does not exist before calling. + * @param int $wait + * Wait a random amount of time between $wait and 2 * $wait microseconds + * between retries. + * @param int $retry + * The number of times to retry. + * + * @return + * TRUE on success, FALSE otherwise. */ - public function load($name); + public function load($name, callable $success = NULL, $force = FALSE, $wait = 200000, $retry = 5); + + /** + * Loads a class from storage. + * + * This function may be called for a file that doesn't exist, and that should + * not result in errors. + * + * It is safe to call this function multiple times: the class will only be + * loaded once. + * + * @param string $name + * The virtual file name. Can be a relative path. + * @param string $class_name + * A race condition might cause the file to exist but be empty. The system + * will retry loading if the class does not exist to allow the racing + * process to finish. + * @param bool $force + * Override the safety guarantee and force loading. For example, the caller + * might've already determined the class does not exist before calling. + * @param int $wait + * Wait a random amount of time between $wait and 2 * $wait microseconds + * between retries. + * @param int $retry + * The number of times to retry. + * + * @return + * TRUE on success, FALSE otherwise. + */ + public function loadClass($name, $class_name, $force = FALSE, $wait = 200000, $retry = 5); /** * Saves PHP code to storage. diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index 8f052ba..03c6a7f 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -742,12 +742,11 @@ 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()->loadClass($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)) { $container = new $fully_qualified_class_name; diff --git a/core/lib/Drupal/Core/Template/TwigEnvironment.php b/core/lib/Drupal/Core/Template/TwigEnvironment.php index 9225712..c30ca2e 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, TRUE)) { $this->updateCompiledTemplate($cache_filename, $name); - $this->storage()->load($cache_filename); + $this->storage()->loadClass($cache_filename, $cls, TRUE); } } } 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.