Permalink
Browse files

MDL-36466 cache: improved handling of exceptions during initialisation.

  • Loading branch information...
1 parent f42c34a commit 7383a7e2a0139fa162544fe7aae50ef69486ea3a Sam Hemelryk committed Nov 12, 2012
Showing with 127 additions and 13 deletions.
  1. +7 −3 cache/classes/config.php
  2. +73 −1 cache/classes/factory.php
  3. +41 −8 cache/locallib.php
  4. +6 −1 lib/setuplib.php
@@ -122,12 +122,15 @@ protected static function get_config_file_path() {
/**
* 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 @@ public function get_stores($mode, $requirements = 0) {
*/
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.
@@ -40,6 +40,19 @@
*/
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
@@ -83,6 +96,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.
*
* @param bool $forcereload If set to true a new cache_factory instance will be created and used.
@@ -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 @@ public static function reset() {
$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 @@ public function create_config_instance($writer = false) {
$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 @@ public function create_config_instance($writer = false) {
$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 @@ public function create_lock_instance(array $config) {
$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;
+ }
}
View
@@ -42,6 +42,14 @@
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.
*
* @return cache_config_writer
@@ -53,6 +61,10 @@ public static function instance() {
/**
* 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 @@ protected function config_save() {
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 = "<?php defined('MOODLE_INTERNAL') || die();\n \$configuration = ".var_export($configuration, true).";";
@@ -107,6 +114,20 @@ protected function config_save() {
}
/**
+ * Generates a configuration array suitable to be written to the config file.
+ * @return array
+ */
+ protected function generate_configuration_array() {
+ $configuration = array();
+ $configuration['stores'] = $this->configstores;
+ $configuration['modemappings'] = $this->configmodemappings;
+ $configuration['definitions'] = $this->configdefinitions;
+ $configuration['definitionmappings'] = $this->configdefinitionmappings;
+ $configuration['locks'] = $this->configlocks;
+ return $configuration;
+ }
+
+ /**
* Adds a plugin instance.
*
* This function also calls save so you should redirect immediately, or at least very shortly after
@@ -293,6 +314,9 @@ public function delete_store_instance($name) {
*
* 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 @@ public static function create_default_configuration() {
'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;
}
/**
View
@@ -1444,7 +1444,12 @@ public static function early_error_content($message, $moreinfourl, $link, $backt
width: 80%; -moz-border-radius: 20px; padding: 15px">
' . $message . '
</div>';
- 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", '<br />', $debuginfo); // keep newlines

0 comments on commit 7383a7e

Please sign in to comment.