CRM-17846 Fixing lock name to be less than 64 characters for MySQL 5.7 compatib...

* Fixing lock name to be less than 64 characters for MySQL 5.7 compatibility

Removing spaces to make Jenkins happy

Updating Lock to keep the name param and add an id to use for the actual lock

Fixing spacing/comment issues

Using lock id instead of  when checking if lock used

Changing behaviour when a lock is attempted using an existing name so as to return FALSE and leave the _hasLock FALSE

Revert "Changing behaviour when a lock is attempted using an existing name so as to return FALSE and leave the _hasLock FALSE"

This reverts commit 4fcb4d2f5b161c69a4f86f09ac363e97791f023e.

* CRM-17846 further fix to make lock mechanism work

* CRM-18746 add Code Comments

diff --git a/CRM/Core/Lock.php b/CRM/Core/Lock.php
index 9b4a418ac17f8b450b40eddce33d68bc3c0ff3c6..682f674e6deda8cca02efd5d2ddb8ee959f9e207 100644 (file)
--- a/CRM/Core/Lock.php
+++ b/CRM/Core/Lock.php
@@ -34,6 +34,16 @@
  */
 class CRM_Core_Lock implements \Civi\Core\Lock\LockInterface {
 
+  /**
+   * This variable (despite it's name) roughly translates to 'lock that we actually care about'.
+   *
+   * Prior to version 5.7.5 mysql only supports a single named lock. This variable is
+   * part of the skullduggery involved in 'say it's no so Frank'.
+   *
+   * See further comments on the aquire function.
+   *
+   * @var bool
+   */
   static $jobLog = FALSE;
 
   // lets have a 3 second timeout for now
@@ -43,6 +53,8 @@ class CRM_Core_Lock implements \Civi\Core\Lock\LockInterface {
 
   protected $_name;
 
+  protected $_id;
+
   /**
    * Use MySQL's GET_LOCK(). Locks are shared across all Civi instances
    * on the same MySQL server.
@@ -121,8 +133,10 @@ class CRM_Core_Lock implements \Civi\Core\Lock\LockInterface {
     else {
       $this->_name = $database . '.' . $domainID . '.' . $name;
     }
+    // MySQL 5.7 doesn't like long lock names so creating a lock id
+    $this->_id = sha1($this->_name);
     if (defined('CIVICRM_LOCK_DEBUG')) {
-      CRM_Core_Error::debug_log_message('trying to construct lock for ' . $this->_name);
+      CRM_Core_Error::debug_log_message('trying to construct lock for ' . $this->_name . '(' . $this->_id . ')');
     }
     $this->_timeout = $timeout !== NULL ? $timeout : self::TIMEOUT;
   }
@@ -134,6 +148,28 @@ class CRM_Core_Lock implements \Civi\Core\Lock\LockInterface {
   /**
    * Acquire lock.
    *
+   * The advantage of mysql locks is that they can be used across processes. However, only one
+   * can be used at once within a process. An attempt to use a second one within a process
+   * prior to mysql 5.7.5 results in the first being released.
+   *
+   * The process here is
+   *  1) first attempt to grab a lock for a mailing job - self::jobLog will be populated with the
+   * lock id & a mysql lock will be created for the ID.
+   *
+   * If a second function in the same process attempts to grab the lock it will enter the hackyHandleBrokenCode routine
+   * which says 'I won't break a mailing lock for you but if you are not a civimail send process I'll let you
+   * pretend you have a lock already and you can go ahead with whatever you were doing under the delusion you
+   * have a lock.
+   *
+   * @todo bypass hackyHandleBrokenCode for mysql version 5.7.5+
+   *
+   * If a second function in a separate process attempts to grab the lock already in use it should be rejected,
+   * but it appears it IS allowed to grab a different lock & unlike in the same process the first lock won't be released.
+   *
+   * All this means CiviMail locks are first class citizens & any other process gets a 'best effort lock'.
+   *
+   * @todo document naming convention for CiviMail locks as this is key to ensuring they work properly.
+   *
    * @param int $timeout
    *
    * @return bool
@@ -147,22 +183,22 @@ class CRM_Core_Lock implements \Civi\Core\Lock\LockInterface {
 
       $query = "SELECT GET_LOCK( %1, %2 )";
       $params = array(
-        1 => array($this->_name, 'String'),
+        1 => array($this->_id, 'String'),
         2 => array($timeout ? $timeout : $this->_timeout, 'Integer'),
       );
       $res = CRM_Core_DAO::singleValueQuery($query, $params);
       if ($res) {
         if (defined('CIVICRM_LOCK_DEBUG')) {
-          CRM_Core_Error::debug_log_message('acquire lock for ' . $this->_name);
+          CRM_Core_Error::debug_log_message('acquire lock for ' . $this->_name . '(' . $this->_id . ')');
         }
         $this->_hasLock = TRUE;
         if (stristr($this->_name, 'data.mailing.job.')) {
-          self::$jobLog = $this->_name;
+          self::$jobLog = $this->_id;
         }
       }
       else {
         if (defined('CIVICRM_LOCK_DEBUG')) {
-          CRM_Core_Error::debug_log_message('failed to acquire lock for ' . $this->_name);
+          CRM_Core_Error::debug_log_message('failed to acquire lock for ' . $this->_name . '(' . $this->_id . ')');
         }
       }
     }
@@ -175,16 +211,16 @@ class CRM_Core_Lock implements \Civi\Core\Lock\LockInterface {
   public function release() {
     if ($this->_hasLock) {
       if (defined('CIVICRM_LOCK_DEBUG')) {
-        CRM_Core_Error::debug_log_message('release lock for ' . $this->_name);
+        CRM_Core_Error::debug_log_message('release lock for ' . $this->_name . '(' . $this->_id . ')');
       }
       $this->_hasLock = FALSE;
 
-      if (self::$jobLog == $this->_name) {
+      if (self::$jobLog == $this->_id) {
         self::$jobLog = FALSE;
       }
 
       $query = "SELECT RELEASE_LOCK( %1 )";
-      $params = array(1 => array($this->_name, 'String'));
+      $params = array(1 => array($this->_id, 'String'));
       return CRM_Core_DAO::singleValueQuery($query, $params);
     }
   }
@@ -194,7 +230,7 @@ class CRM_Core_Lock implements \Civi\Core\Lock\LockInterface {
    */
   public function isFree() {
     $query = "SELECT IS_FREE_LOCK( %1 )";
-    $params = array(1 => array($this->_name, 'String'));
+    $params = array(1 => array($this->_id, 'String'));
     return CRM_Core_DAO::singleValueQuery($query, $params);
   }
 
@@ -220,11 +256,11 @@ class CRM_Core_Lock implements \Civi\Core\Lock\LockInterface {
    */
   public function hackyHandleBrokenCode($jobLog) {
     if (stristr($this->_name, 'job')) {
-      CRM_Core_Error::debug_log_message('lock acquisition for ' . $this->_name . ' attempted when ' . $jobLog . ' is not released');
-      throw new CRM_Core_Exception('lock acquisition for ' . $this->_name . ' attempted when ' . $jobLog . ' is not released');
+      CRM_Core_Error::debug_log_message('lock acquisition for ' . $this->_name . '(' . $this->_id . ')' . ' attempted when ' . $jobLog . ' is not released');
+      throw new CRM_Core_Exception('lock acquisition for ' . $this->_name . '(' . $this->_id . ')' . ' attempted when ' . $jobLog . ' is not released');
     }
     if (defined('CIVICRM_LOCK_DEBUG')) {
-      CRM_Core_Error::debug_log_message('(CRM-12856) faking lock for ' . $this->_name);
+      CRM_Core_Error::debug_log_message('(CRM-12856) faking lock for ' . $this->_name . '(' . $this->_id . ')');
     }
     $this->_hasLock = TRUE;
     return TRUE;