MDL-36466 cache: improved handling of exceptions during initialisation.
authorSam Hemelryk <sam@moodle.com>
Mon, 12 Nov 2012 20:26:07 +0000 (09:26 +1300)
committerSam Hemelryk <sam@moodle.com>
Sun, 25 Nov 2012 19:11:59 +0000 (08:11 +1300)
cache/classes/config.php
cache/classes/factory.php
cache/locallib.php
lib/setuplib.php

index da3b966..eb0ef6f 100644 (file)
@@ -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.
index 9a588df..cadce01 100644 (file)
@@ -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
index e8e43a3..5e3cc11 100644 (file)
@@ -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 = "<?php defined('MOODLE_INTERNAL') || die();\n \$configuration = ".var_export($configuration, true).";";
@@ -106,6 +113,20 @@ class cache_config_writer extends cache_config {
         }
     }
 
+    /**
+     * 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.
      *
@@ -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;
     }
 
     /**
index d100654..01c2f58 100644 (file)
@@ -1444,7 +1444,12 @@ border-color:black; background-color:#ffffee; border-style:solid; border-radius:
 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