From abae2129ad4e22511d15686d5446cb23d8ea260a Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Mon, 26 Aug 2013 15:29:32 +0000 Subject: [PATCH] CRON: protection against re-entrance now relies on a bullet-proof mutex. Also added the option 'debug=1' to output the call stack in case an exception occurs (not always because of passwords being shown in the call stack) SVN:trunk[2834] --- core/mutex.class.inc.php | 138 +++++++++++++++++++++++++++++++++++++++ webservices/cron.php | 48 ++++++-------- 2 files changed, 159 insertions(+), 27 deletions(-) create mode 100644 core/mutex.class.inc.php diff --git a/core/mutex.class.inc.php b/core/mutex.class.inc.php new file mode 100644 index 000000000..1df42f6a8 --- /dev/null +++ b/core/mutex.class.inc.php @@ -0,0 +1,138 @@ + + + +/** + * Class iTopMutex + * A class to serialize the execution of some code sections + * Emulates the API of PECL Mutex class + * Relies on MySQL locks because the API sem_get is not always present in the + * installed PHP. + * + * @copyright Copyright (C) 2013 Combodo SARL + * @license http://opensource.org/licenses/AGPL-3.0 + */ +class iTopMutex +{ + protected $sName; + protected $hDBLink; + + public function __construct($sName) + { + // Compute the name of a lock for mysql + // Note: the name is server-wide!!! + $this->sName = 'itop.'.$sName; + + // It is a MUST to create a dedicated session each time a lock is required, because + // using GET_LOCK anytime on the same session will RELEASE the current and unique session lock (known issue) + $oConfig = utils::GetConfig(); + $this->InitMySQLSession($oConfig->GetDBHost(), $oConfig->GetDBUser(), $oConfig->GetDBPwd()); + } + + public function __destruct() + { + $this->Unlock(); + mysqli_close($this->hDBLink); + } + + /** + * Acquire the mutex + */ + public function Lock() + { + do + { + $res = $this->QueryToScalar("SELECT GET_LOCK('".$this->sName."', 3600)"); + if (is_null($res)) + { + throw new Exception("Failed to acquire the lock '".$this->sName."'"); + } + // $res === '1' means I hold the lock + // $res === '0' means it timed out + } + while ($res !== '1'); + } + + /** + * Attempt to acquire the mutex + * @returns bool True if the mutex is acquired, false if already locked elsewhere + */ + public function TryLock() + { + $res = $this->QueryToScalar("SELECT GET_LOCK('".$this->sName."', 0)"); + if (is_null($res)) + { + throw new Exception("Failed to acquire the lock '".$this->sName."'"); + } + // $res === '1' means I hold the lock + // $res === '0' means it timed out + return ($res === '1'); + } + + /** + * Release the mutex + */ + public function Unlock() + { + $res = $this->QueryToScalar("SELECT RELEASE_LOCK('".$this->sName."')"); + } + + + + public function InitMySQLSession($sHost, $sUser, $sPwd) + { + $aConnectInfo = explode(':', $sHost); + if (count($aConnectInfo) > 1) + { + // Override the default port + $sServer = $aConnectInfo[0]; + $iPort = $aConnectInfo[1]; + $this->hDBLink = @mysqli_connect($sServer, $sUser, $sPwd, '', $iPort); + } + else + { + $this->hDBLink = @mysqli_connect($sHost, $sUser, $sPwd); + } + + if (!$this->hDBLink) + { + throw new Exception("Could not connect to the DB server (host=$sHost, user=$sUser): ".mysqli_connect_error().' (mysql errno: '.mysqli_connect_errno().')'); + } + } + + + protected function QueryToScalar($sSql) + { + $result = mysqli_query($this->hDBLink, $sSql); + if (!$result) + { + throw new Exception("Failed to issue MySQL query '".$sSql."': ".mysqli_error($this->hDBLink).' (mysql errno: '.mysqli_errno($this->hDBLink).')'); + } + if ($aRow = mysqli_fetch_array($result, MYSQLI_BOTH)) + { + $res = $aRow[0]; + } + else + { + mysqli_free_result($result); + throw new Exception("No result for query '".$sSql."'"); + } + mysqli_free_result($result); + return $res; + } +} diff --git a/webservices/cron.php b/webservices/cron.php index 1e96e2d76..f2818678b 100644 --- a/webservices/cron.php +++ b/webservices/cron.php @@ -19,7 +19,7 @@ /** * Heart beat of the application (process asynchron tasks such as broadcasting email) * - * @copyright Copyright (C) 2010-2012 Combodo SARL + * @copyright Copyright (C) 2010-2013 Combodo SARL * @license http://opensource.org/licenses/AGPL-3.0 */ @@ -59,7 +59,7 @@ function UsageAndExit($oP) if ($bModeCLI) { $oP->p("USAGE:\n"); - $oP->p("php cron.php --auth_user= --auth_pwd= [--param_file=] [--verbose=1] [--status_only=1]\n"); + $oP->p("php cron.php --auth_user= --auth_pwd= [--param_file=] [--verbose=1] [--debug=1] [--status_only=1]\n"); } else { @@ -337,6 +337,7 @@ foreach(get_declared_classes() as $sPHPClass) $bVerbose = utils::ReadParam('verbose', false, true /* Allow CLI */); +$bDebug = utils::ReadParam('debug', false, true /* Allow CLI */); if ($bVerbose) { @@ -355,42 +356,35 @@ if (utils::ReadParam('status_only', false, true /* Allow CLI */)) exit(0); } -// Compute the name of a lock for mysql -// The name is server-wide -$oConfig = utils::GetConfig(); -$sLockName = 'itop.cron.'.$oConfig->GetDBName().'_'.$oConfig->GetDBSubname(); - +require_once(APPROOT.'core/mutex.class.inc.php'); $oP->p("Starting: ".time().' ('.date('Y-m-d H:i:s').')'); -// CAUTION: using GET_LOCK anytime on the same connexion will RELEASE the lock -// Todo: invoke GET_LOCK from a dedicated session (encapsulate that into a mutex class) -$res = CMDBSource::QueryToScalar("SELECT GET_LOCK('$sLockName', 1)");// timeout = 1 second (see also IS_FREE_LOCK) -if (is_null($res)) +try { - // TODO - Log ? - $oP->p("ERROR: Failed to acquire the lock '$sLockName'"); -} -elseif ($res === '1') -{ - // The current session holds the lock - try + $oConfig = utils::GetConfig(); + $oMutex = new iTopMutex('cron.'.$oConfig->GetDBName().'_'.$oConfig->GetDBSubname()); + if ($oMutex->TryLock()) { CronExec($oP, $aProcesses, $bVerbose); + + $oMutex->Unlock(); } - catch(Exception $e) + else { - // TODO - Log ? - $oP->p("ERROR:".$e->getMessage()); - $oP->p($e->getTraceAsString()); + // Exit silently + $oP->p("Already running..."); } - $res = CMDBSource::QueryToScalar("SELECT RELEASE_LOCK('$sLockName')"); } -else +catch (Exception $e) { - // Lock already held by another session - // Exit silently - $oP->p("Already running..."); + $oP->p("ERROR: '".$e->getMessage()."'"); + if ($bDebug) + { + // Might contain verb parameters such a password... + $oP->p($e->getTraceAsString()); + } } + $oP->p("Exiting: ".time().' ('.date('Y-m-d H:i:s').')'); $oP->Output();