From 7383a7e2a0139fa162544fe7aae50ef69486ea3a Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Tue, 13 Nov 2012 09:26:07 +1300 Subject: [PATCH] MDL-36466 cache: improved handling of exceptions during initialisation. --- cache/classes/config.php | 10 ++++-- cache/classes/factory.php | 74 ++++++++++++++++++++++++++++++++++++++- cache/locallib.php | 49 +++++++++++++++++++++----- lib/setuplib.php | 7 +++- 4 files changed, 127 insertions(+), 13 deletions(-) diff --git a/cache/classes/config.php b/cache/classes/config.php index da3b9665915..eb0ef6f06ca 100644 --- a/cache/classes/config.php +++ b/cache/classes/config.php @@ -122,12 +122,15 @@ class cache_config { /** * Loads the configuration file and parses its contents into the expected structure. * + * @param array|false $configuration Can be used to force a configuration. Should only be used when truly required. * @return boolean */ - public function load() { + public function load($configuration = false) { global $CFG; - $configuration = $this->include_configuration(); + if ($configuration === false) { + $configuration = $this->include_configuration(); + } $this->configstores = array(); $this->configdefinitions = array(); @@ -425,7 +428,8 @@ class cache_config { */ public function get_stores_for_definition(cache_definition $definition) { // Check if MUC has been disabled. - if (defined('NO_CACHE_STORES') && NO_CACHE_STORES !== false) { + $factory = cache_factory::instance(); + if ($factory->is_disabled()) { // Yip its been disabled. // To facilitate this we are going to always return an empty array of stores to use. // This will force all cache instances to use the cachestore_dummy. diff --git a/cache/classes/factory.php b/cache/classes/factory.php index 9a588dfe794..cadce01554c 100644 --- a/cache/classes/factory.php +++ b/cache/classes/factory.php @@ -40,6 +40,19 @@ defined('MOODLE_INTERNAL') || die(); */ class cache_factory { + /** The cache has not been initialised yet. */ + const STATE_UNINITIALISED = 0; + /** The cache is in the process of initialising itself. */ + const STATE_INITIALISING = 1; + /** The cache is in the process of saving its configuration file. */ + const STATE_SAVING = 2; + /** The cache is ready to use. */ + const STATE_READY = 3; + /** The cache encountered an error while initialising. */ + const STATE_ERROR_INITIALISING = 9; + /** The cache has been disabled. */ + const STATE_DISABLED = 10; + /** * An instance of the cache_factory class created upon the first request. * @var cache_factory @@ -82,6 +95,12 @@ class cache_factory { */ protected $lockplugins = null; + /** + * The current state of the cache API. + * @var int + */ + protected $state = 0; + /** * Returns an instance of the cache_factor method. * @@ -91,6 +110,10 @@ class cache_factory { public static function instance($forcereload = false) { if ($forcereload || self::$instance === null) { self::$instance = new cache_factory(); + if (defined('NO_CACHE_STORES') && NO_CACHE_STORES !== false) { + // The cache stores have been disabled. + self::$instance->set_state(self::STATE_DISABLED);; + } } return self::$instance; } @@ -113,6 +136,8 @@ class cache_factory { $factory->configs = array(); $factory->definitions = array(); $factory->lockplugins = null; // MUST be null in order to force its regeneration. + // Reset the state to uninitialised. + $factory->state = self::STATE_UNINITIALISED; } /** @@ -253,9 +278,19 @@ class cache_factory { $class = 'cache_config_phpunittest'; } + $error = false; if ($needtocreate) { // Create the default configuration. - $class::create_default_configuration(); + // Update the state, we are now initialising the cache. + self::set_state(self::STATE_INITIALISING); + $configuration = $class::create_default_configuration(); + if ($configuration !== true) { + // Failed to create the default configuration. Disable the cache stores and update the state. + self::set_state(self::STATE_ERROR_INITIALISING); + $this->configs[$class] = new $class; + $this->configs[$class]->load($configuration); + $error = true; + } } if (!array_key_exists($class, $this->configs)) { @@ -264,6 +299,11 @@ class cache_factory { $this->configs[$class]->load(); } + if (!$error) { + // The cache is now ready to use. Update the state. + self::set_state(self::STATE_READY); + } + // Return the instance. return $this->configs[$class]; } @@ -338,4 +378,36 @@ class cache_factory { $class = $this->lockplugins[$type]; return new $class($name, $config); } + + /** + * Returns the current state of the cache API. + * + * @return int + */ + public function get_state() { + return $this->state; + } + + /** + * Updates the state fo the cache API. + * + * @param int $state + * @return bool + */ + public function set_state($state) { + if ($state <= $this->state) { + return false; + } + $this->state = $state; + return true; + } + + /** + * Returns true if the cache API has been disabled. + * + * @return bool + */ + public function is_disabled() { + return $this->state === self::STATE_DISABLED; + } } \ No newline at end of file diff --git a/cache/locallib.php b/cache/locallib.php index e8e43a3d454..5e3cc1126cf 100644 --- a/cache/locallib.php +++ b/cache/locallib.php @@ -41,6 +41,14 @@ defined('MOODLE_INTERNAL') || die(); */ class cache_config_writer extends cache_config { + /** + * Switch that gets set to true when ever a cache_config_writer instance is saving the cache configuration file. + * If this is set to true when save is next called we must avoid the trying to save and instead return the + * generated config so that is may be used instead of the file. + * @var bool + */ + protected static $creatingconfig = false; + /** * Returns an instance of the configuration writer. * @@ -53,6 +61,10 @@ class cache_config_writer extends cache_config { /** * Saves the current configuration. + * + * Exceptions within this function are tolerated but must be of type cache_exception. + * They are caught during initialisation and written to the error log. This is required in order to avoid + * infinite loop situations caused by the cache throwing exceptions during its initialisation. */ protected function config_save() { global $CFG; @@ -61,20 +73,15 @@ class cache_config_writer extends cache_config { if ($directory !== $CFG->dataroot && !file_exists($directory)) { $result = make_writable_directory($directory, false); if (!$result) { - throw new cache_exception('ex_configcannotsave', 'cache', '', null, 'Cannot create config directory.'); + throw new cache_exception('ex_configcannotsave', 'cache', '', null, 'Cannot create config directory. Check the permissions on your moodledata directory.'); } } if (!file_exists($directory) || !is_writable($directory)) { - throw new cache_exception('ex_configcannotsave', 'cache', '', null, 'Config directory is not writable.'); + throw new cache_exception('ex_configcannotsave', 'cache', '', null, 'Config directory is not writable. Check the permissions on the moodledata/muc directory.'); } // Prepare a configuration array to store. - $configuration = array(); - $configuration['stores'] = $this->configstores; - $configuration['modemappings'] = $this->configmodemappings; - $configuration['definitions'] = $this->configdefinitions; - $configuration['definitionmappings'] = $this->configdefinitionmappings; - $configuration['locks'] = $this->configlocks; + $configuration = $this->generate_configuration_array(); // Prepare the file content. $content = "configstores; + $configuration['modemappings'] = $this->configmodemappings; + $configuration['definitions'] = $this->configdefinitions; + $configuration['definitionmappings'] = $this->configdefinitionmappings; + $configuration['locks'] = $this->configlocks; + return $configuration; + } + /** * Adds a plugin instance. * @@ -293,6 +314,9 @@ class cache_config_writer extends cache_config { * * This function calls config_save, however it is safe to continue using it afterwards as this function should only ever * be called when there is no configuration file already. + * + * @return true|array Returns true if the default configuration was successfully created. + * Returns a configuration array if it could not be saved. This is a bad situation. Check your error logs. */ public static function create_default_configuration() { global $CFG; @@ -357,7 +381,16 @@ class cache_config_writer extends cache_config { 'default' => true ) ); + + $factory = cache_factory::instance(); + // We expect the cache to be initialising presently. If its not then something has gone wrong and likely + // we are now in a loop. + if ($factory->get_state() !== cache_factory::STATE_INITIALISING) { + return $writer->generate_configuration_array(); + } + $factory->set_state(cache_factory::STATE_SAVING); $writer->config_save(); + return true; } /** diff --git a/lib/setuplib.php b/lib/setuplib.php index d100654f954..01c2f580282 100644 --- a/lib/setuplib.php +++ b/lib/setuplib.php @@ -1444,7 +1444,12 @@ border-color:black; background-color:#ffffee; border-style:solid; border-radius: width: 80%; -moz-border-radius: 20px; padding: 15px"> ' . $message . ' '; - if (!empty($CFG->debug) && $CFG->debug >= DEBUG_DEVELOPER) { + // Check whether debug is set. + $debug = (!empty($CFG->debug) && $CFG->debug >= DEBUG_DEVELOPER); + // Also check we have it set in the config file. This occurs if the method to read the config table from the + // database fails, reading from the config table is the first database interaction we have. + $debug = $debug || (!empty($CFG->config_php_settings['debug']) && $CFG->config_php_settings['debug'] >= DEBUG_DEVELOPER ); + if ($debug) { if (!empty($debuginfo)) { $debuginfo = s($debuginfo); // removes all nasty JS $debuginfo = str_replace("\n", '
', $debuginfo); // keep newlines