From 836178864b970fe12e529f1f3e43047cfa185d68 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Fri, 18 Sep 2015 22:09:01 -0700 Subject: [PATCH] CRM-16373 - Bootstrap subsystems in more predictable order 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 | 2 +- CRM/Core/BAO/Cache.php | 2 +- CRM/Core/Config.php | 5 +++ CRM/Core/Config/Runtime.php | 24 ++++++++++ CRM/Mailing/BAO/Mailing.php | 2 +- CRM/Mailing/BAO/MailingJob.php | 4 +- Civi.php | 12 ++++- Civi/Core/Container.php | 65 ++++++++++++++++++--------- api/v3/Job.php | 10 ++--- bin/ContributionProcessor.php | 2 +- 10 files changed, 93 insertions(+), 35 deletions(-) diff --git a/CRM/Contact/BAO/GroupContactCache.php b/CRM/Contact/BAO/GroupContactCache.php index 8bbaa3ff78..74fbe77fe1 100644 --- a/CRM/Contact/BAO/GroupContactCache.php +++ b/CRM/Contact/BAO/GroupContactCache.php @@ -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. diff --git a/CRM/Core/BAO/Cache.php b/CRM/Core/BAO/Cache.php index cb504ed9dd..3652934971 100644 --- a/CRM/Core/BAO/Cache.php +++ b/CRM/Core/BAO/Cache.php @@ -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(); } diff --git a/CRM/Core/Config.php b/CRM/Core/Config.php index 33eec9344b..4bb8f5cb20 100644 --- a/CRM/Core/Config.php +++ b/CRM/Core/Config.php @@ -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); diff --git a/CRM/Core/Config/Runtime.php b/CRM/Core/Config/Runtime.php index 95f4810d72..bfaf7c08e6 100644 --- a/CRM/Core/Config/Runtime.php +++ b/CRM/Core/Config/Runtime.php @@ -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']; + } + + } diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index 6c134c8b22..9d123ab3f0 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -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; diff --git a/CRM/Mailing/BAO/MailingJob.php b/CRM/Mailing/BAO/MailingJob.php index 5ccb3a94b2..a1a232c16b 100644 --- a/CRM/Mailing/BAO/MailingJob.php +++ b/CRM/Mailing/BAO/MailingJob.php @@ -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; } diff --git a/Civi.php b/Civi.php index 01e1fbbc4d..82f738f271 100644 --- 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); } } diff --git a/Civi/Core/Container.php b/Civi/Core/Container.php index 5488a44567..e75fa4c3f9 100644 --- a/Civi/Core/Container.php +++ b/Civi/Core/Container.php @@ -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']; + } + } diff --git a/api/v3/Job.php b/api/v3/Job.php index 2d953119c8..8bc0de5216 100644 --- a/api/v3/Job.php +++ b/api/v3/Job.php @@ -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'); } diff --git a/bin/ContributionProcessor.php b/bin/ContributionProcessor.php index 538ac0dfd4..ae976e7e9a 100644 --- a/bin/ContributionProcessor.php +++ b/bin/ContributionProcessor.php @@ -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 -- 2.25.1