CRM-16373 - Bootstrap subsystems in more predictable order
authorTim Otten <totten@civicrm.org>
Sat, 19 Sep 2015 05:09:01 +0000 (22:09 -0700)
committerTim Otten <totten@civicrm.org>
Mon, 21 Sep 2015 04:24:28 +0000 (21:24 -0700)
Several subsystems were initialized on-demand. This had a weird side-effect
where the container would start initializing and call another service, which
would then (indirectly) try to initialize another container. The two container
instances would not be the same.

This revision enforces a more carefully considered sequence of events (which
will be documented in the wiki page, "Bootstrap Reference").

CRM/Contact/BAO/GroupContactCache.php
CRM/Core/BAO/Cache.php
CRM/Core/Config.php
CRM/Core/Config/Runtime.php
CRM/Mailing/BAO/Mailing.php
CRM/Mailing/BAO/MailingJob.php
Civi.php
Civi/Core/Container.php
api/v3/Job.php
bin/ContributionProcessor.php

index 8bbaa3ff78d7f58837030bc022222d34ea648e64..74fbe77fe1eede7d2aa66b21d85149698a673cfa 100644 (file)
@@ -431,7 +431,7 @@ WHERE  id = %1
     }
 
     // grab a lock so other processes dont compete and do the same query
-    $lock = Civi::service('lockManager')->acquire("data.core.group.{$groupID}");
+    $lock = Civi::lockManager()->acquire("data.core.group.{$groupID}");
     if (!$lock->isAcquired()) {
       // this can cause inconsistent results since we dont know if the other process
       // will fill up the cache before our calling routine needs it.
index cb504ed9dd55e261fd3c0173f54116f17f31a5fa..36529349710bd83ffe224e35b598d3f00075e194 100644 (file)
@@ -153,7 +153,7 @@ class CRM_Core_BAO_Cache extends CRM_Core_DAO_Cache {
     // get a lock so that multiple ajax requests on the same page
     // dont trample on each other
     // CRM-11234
-    $lock = Civi::service('lockManager')->acquire("cache.{$group}_{$path}._{$componentID}");
+    $lock = Civi::lockManager()->acquire("cache.{$group}_{$path}._{$componentID}");
     if (!$lock->isAcquired()) {
       CRM_Core_Error::fatal();
     }
index 33eec9344ba282b92e46a08b11ede8eadcb5fd8d..4bb8f5cb204d34abea0a469b71065e8b5f612cca 100644 (file)
@@ -90,6 +90,11 @@ class CRM_Core_Config extends CRM_Core_Config_MagicMerge {
       self::$_singleton->getRuntime()->initialize($loadFromDB);
       if ($loadFromDB && self::$_singleton->getRuntime()->dsn) {
         CRM_Core_DAO::init(self::$_singleton->getRuntime()->dsn);
+      }
+      \Civi\Core\Container::getBootServices();
+      if ($loadFromDB && self::$_singleton->getRuntime()->dsn) {
+        CRM_Extension_System::singleton();
+        \Civi\Core\Container::singleton();
 
         $domain = \CRM_Core_BAO_Domain::getDomain();
         \CRM_Core_BAO_ConfigSetting::applyLocale(\Civi::settings($domain->id), $domain->locales);
index 95f4810d7235da242aa41f663bf54a89af6af3b1..bfaf7c08e651b79c3f62cd83654002cce4ed45c8 100644 (file)
@@ -195,4 +195,28 @@ class CRM_Core_Config_Runtime {
     exit();
   }
 
+  /**
+   * Create a unique identification code for this runtime.
+   *
+   * If two requests involve a different hostname, different
+   * port, different DSN, etc., then they should also have a
+   * different runtime ID.
+   *
+   * @return mixed
+   */
+  public static function getId() {
+    if (!isset(Civi::$statics[__CLASS__]['id'])) {
+      Civi::$statics[__CLASS__]['id'] = md5(implode(\CRM_Core_DAO::VALUE_SEPARATOR, array(
+        defined('CIVICRM_DOMAIN_ID') ? CIVICRM_DOMAIN_ID : 1, // e.g. one database, multi URL
+        parse_url(CIVICRM_DSN, PHP_URL_PATH), // e.g. one codebase, multi database
+        \CRM_Utils_Array::value('SCRIPT_FILENAME', $_SERVER, ''), // e.g. CMS vs extern vs installer
+        \CRM_Utils_Array::value('HTTP_HOST', $_SERVER, ''), // e.g. name-based vhosts
+        \CRM_Utils_Array::value('SERVER_PORT', $_SERVER, ''), // e.g. port-based vhosts
+        // Depending on deployment arch, these signals *could* be redundant, but who cares?
+      )));
+    }
+    return Civi::$statics[__CLASS__]['id'];
+  }
+
+
 }
index 6c134c8b222f2606ec7d0ff9567e97a26176e397..9d123ab3f0884d7cff64cada15d4d69b3580e8cb 100644 (file)
@@ -2854,7 +2854,7 @@ WHERE  civicrm_mailing_job.id = %1
 
       // check if we are using global locks
       foreach ($lockArray as $lockID) {
-        $cronLock = Civi::service('lockManager')->acquire("worker.mailing.send.{$lockID}");
+        $cronLock = Civi::lockManager()->acquire("worker.mailing.send.{$lockID}");
         if ($cronLock->isAcquired()) {
           $gotCronLock = TRUE;
           break;
index 5ccb3a94b2cce7643da9fed2493794f98883e7e5..a1a232c16b21affe1509870bb4f00781c47d699e 100644 (file)
@@ -136,7 +136,7 @@ class CRM_Mailing_BAO_MailingJob extends CRM_Mailing_DAO_MailingJob {
 
     while ($job->fetch()) {
       // still use job level lock for each child job
-      $lock = Civi::service('lockManager')->acquire("data.mailing.job.{$job->id}");
+      $lock = Civi::lockManager()->acquire("data.mailing.job.{$job->id}");
       if (!$lock->isAcquired()) {
         continue;
       }
@@ -341,7 +341,7 @@ class CRM_Mailing_BAO_MailingJob extends CRM_Mailing_DAO_MailingJob {
     // X Number of child jobs
     while ($job->fetch()) {
       // still use job level lock for each child job
-      $lock = Civi::service('lockManager')->acquire("data.mailing.job.{$job->id}");
+      $lock = Civi::lockManager()->acquire("data.mailing.job.{$job->id}");
       if (!$lock->isAcquired()) {
         continue;
       }
index 01e1fbbc4d773db0639c0ea61cd20770d09e91df..82f738f271946a27ce244bc3d8b05954a9eb7570 100644 (file)
--- a/Civi.php
+++ b/Civi.php
@@ -54,6 +54,13 @@ class Civi {
     return Civi\Core\Container::singleton();
   }
 
+  /**
+   * @return \Civi\Core\Lock\LockManager
+   */
+  public static function lockManager() {
+    return \Civi\Core\Container::getBootService('lockManager');
+  }
+
   /**
    * @return \Psr\Log\LoggerInterface
    */
@@ -90,8 +97,9 @@ class Civi {
    * singletons, containers.
    */
   public static function reset() {
-    Civi\Core\Container::singleton(TRUE);
     self::$statics = array();
+    Civi\Core\Container::getBootServices();
+    Civi\Core\Container::singleton(TRUE);
   }
 
   /**
@@ -109,7 +117,7 @@ class Civi {
    * @return \Civi\Core\SettingsBag
    */
   public static function settings($domainID = NULL) {
-    return Civi\Core\Container::singleton()->get('settings_manager')->getBagByDomain($domainID);
+    return \Civi\Core\Container::getBootService('settings_manager')->getBagByDomain($domainID);
   }
 
 }
index 5488a44567d12dceb9bec4fc4d171823f2ef88ba..e75fa4c3f93f86da610c8de46d530480471875dc 100644 (file)
@@ -76,17 +76,9 @@ class Container {
       return $this->createContainer();
     }
 
-    $envId = md5(implode(\CRM_Core_DAO::VALUE_SEPARATOR, array(
-      defined('CIVICRM_DOMAIN_ID') ? CIVICRM_DOMAIN_ID : 1, // e.g. one database, multi URL
-      parse_url(CIVICRM_DSN, PHP_URL_PATH), // e.g. one codebase, multi database
-      \CRM_Utils_Array::value('SCRIPT_FILENAME', $_SERVER, ''), // e.g. CMS vs extern vs installer
-      \CRM_Utils_Array::value('HTTP_HOST', $_SERVER, ''), // e.g. name-based vhosts
-      \CRM_Utils_Array::value('SERVER_PORT', $_SERVER, ''), // e.g. port-based vhosts
-      // Depending on deployment arch, these signals *could* be redundant, but who cares?
-    )));
+    $envId = \CRM_Core_Config_Runtime::getId();
     $file = CIVICRM_TEMPLATE_COMPILEDIR . "/CachedCiviContainer.{$envId}.php";
     $containerConfigCache = new ConfigCache($file, $cacheMode === 'auto');
-
     if (!$containerConfigCache->isFresh()) {
       $containerBuilder = $this->createContainer();
       $containerBuilder->compile();
@@ -137,12 +129,6 @@ class Container {
     //      }
     //    }
 
-    $container->setDefinition('lockManager', new Definition(
-      'Civi\Core\Lock\LockManager',
-      array()
-    ))
-      ->setFactoryService(self::SELF)->setFactoryMethod('createLockManager');
-
     $container->setDefinition('angular', new Definition(
       'Civi\Angular\Manager',
       array()
@@ -174,7 +160,7 @@ class Container {
 
     $container->setDefinition('psr_log', new Definition('CRM_Core_Error_Log', array()));
 
-    foreach (array('settings', 'js_strings', 'community_messages') as $cacheName) {
+    foreach (array('js_strings', 'community_messages') as $cacheName) {
       $container->setDefinition("cache.{$cacheName}", new Definition(
         'CRM_Utils_Cache_Interface',
         array(
@@ -186,14 +172,15 @@ class Container {
       ))->setFactoryClass('CRM_Utils_Cache')->setFactoryMethod('create');
     }
 
-    $container->setDefinition('settings_manager', new Definition(
-      'Civi\Core\SettingsManager',
-      array(new Reference('cache.settings'))
-    ));
-
     $container->setDefinition('pear_mail', new Definition('Mail'))
       ->setFactoryClass('CRM_Utils_Mail')->setFactoryMethod('createMailer');
 
+    foreach (self::getBootServices() as $bootService => $def) {
+      $container->setDefinition($bootService, new Definition($def['class'], array($bootService)))
+        ->setFactoryClass(__CLASS__)
+        ->setFactoryMethod('getBootService');
+    }
+
     // Expose legacy singletons as services in the container.
     $singletons = array(
       'resources' => 'CRM_Core_Resources',
@@ -244,7 +231,7 @@ class Container {
   /**
    * @return LockManager
    */
-  public function createLockManager() {
+  public static function createLockManager() {
     // Ideally, downstream implementers could override any definitions in
     // the container. For now, we'll make-do with some define()s.
     $lm = new LockManager();
@@ -313,4 +300,38 @@ class Container {
     return $kernel;
   }
 
+  /**
+   * Get a list of boot services.
+   *
+   * These are services which must be setup *before* the container can operate.
+   *
+   * @return array
+   *   Ex: $result['serviceName'] = array('class' => $, 'obj' => $).
+   * @throws \CRM_Core_Exception
+   */
+  public static function getBootServices() {
+    if (!isset(\Civi::$statics['*boot*'])) {
+      \Civi::$statics['*boot*']['cache.settings'] = array(
+        'class' => 'CRM_Utils_Cache_Interface',
+        'obj' => \CRM_Utils_Cache::create(array(
+          'name' => 'settings',
+          'type' => array('*memory*', 'SqlGroup', 'ArrayCache'),
+        )),
+      );
+      \Civi::$statics['*boot*']['settings_manager'] = array(
+        'class' => 'Civi\Core\SettingsManager',
+        'obj' => new \Civi\Core\SettingsManager(\Civi::$statics['*boot*']['cache.settings']['obj']),
+      );
+      \Civi::$statics['*boot*']['lockManager'] = array(
+        'class' => 'Civi\Core\Lock\LockManager',
+        'obj' => self::createLockManager(),
+      );
+    }
+    return \Civi::$statics['*boot*'];
+  }
+
+  public static function getBootService($name) {
+    return \Civi::$statics['*boot*'][$name]['obj'];
+  }
+
 }
index 2d953119c86de4fd85f1f055a6516ac4b719f7d1..8bc0de5216cbb6cb151df894a8bb1dd45039d355 100644 (file)
@@ -178,7 +178,7 @@ function civicrm_api3_job_send_reminder($params) {
   // in that case (ie. makes it non-configurable via the UI). Another approach would be to set a default of 0
   // in the _spec function - but since that is a deprecated value it seems more contentious than this approach
   $params['rowCount'] = 0;
-  $lock = Civi::service('lockManager')->acquire('worker.core.ActionSchedule');
+  $lock = Civi::lockManager()->acquire('worker.core.ActionSchedule');
   if (!$lock->isAcquired()) {
     return civicrm_api3_create_error('Could not acquire lock, another ActionSchedule process is running');
   }
@@ -354,7 +354,7 @@ function civicrm_api3_job_process_sms($params) {
  * @return array
  */
 function civicrm_api3_job_fetch_bounces($params) {
-  $lock = Civi::service('lockManager')->acquire('worker.mailing.EmailProcessor');
+  $lock = Civi::lockManager()->acquire('worker.mailing.EmailProcessor');
   if (!$lock->isAcquired()) {
     return civicrm_api3_create_error('Could not acquire lock, another EmailProcessor process is running');
   }
@@ -377,7 +377,7 @@ function civicrm_api3_job_fetch_bounces($params) {
  * @return array
  */
 function civicrm_api3_job_fetch_activities($params) {
-  $lock = Civi::service('lockManager')->acquire('worker.mailing.EmailProcessor');
+  $lock = Civi::lockManager()->acquire('worker.mailing.EmailProcessor');
   if (!$lock->isAcquired()) {
     return civicrm_api3_create_error('Could not acquire lock, another EmailProcessor process is running');
   }
@@ -430,7 +430,7 @@ function civicrm_api3_job_process_participant($params) {
  *   true if success, else false
  */
 function civicrm_api3_job_process_membership($params) {
-  $lock = Civi::service('lockManager')->acquire('worker.member.UpdateMembership');
+  $lock = Civi::lockManager()->acquire('worker.member.UpdateMembership');
   if (!$lock->isAcquired()) {
     return civicrm_api3_create_error('Could not acquire lock, another Membership Processing process is running');
   }
@@ -627,7 +627,7 @@ function civicrm_api3_job_disable_expired_relationships($params) {
  * @throws \API_Exception
  */
 function civicrm_api3_job_group_rebuild($params) {
-  $lock = Civi::service('lockManager')->acquire('worker.core.GroupRebuild');
+  $lock = Civi::lockManager()->acquire('worker.core.GroupRebuild');
   if (!$lock->isAcquired()) {
     throw new API_Exception('Could not acquire lock, another EmailProcessor process is running');
   }
index 538ac0dfd4aa2d90428deb2a265b35421a0c2092..ae976e7e9af10dbbf008299519dffd4c99933524 100644 (file)
@@ -697,7 +697,7 @@ CRM_Utils_System::authenticateScript(TRUE);
 //log the execution of script
 CRM_Core_Error::debug_log_message('ContributionProcessor.php');
 
-$lock = Civi::service('lockManager')->acquire('worker.contribute.CiviContributeProcessor');
+$lock = Civi::lockManager()->acquire('worker.contribute.CiviContributeProcessor');
 
 if ($lock->isAcquired()) {
   // try to unset any time limits