MDL-14992 towards better db sessions - session locking for mysql

diff --git a/lib/dml/moodle_database.php b/lib/dml/moodle_database.php
index 51bb43796917d37a6dd98b837b0a1ffb491def7f..cf1b4cd82f326edc7ca89636c1857825ba274e64 100644 (file)
--- a/lib/dml/moodle_database.php
+++ b/lib/dml/moodle_database.php
@@ -1739,6 +1739,17 @@ abstract class moodle_database {
         return false;
     }
 
+/// session locking
+    public function get_session_lock($name, $timeout) {
+        // TODO: make this abstract
+        return true;
+    }
+
+    public function release_session_lock($name) {
+        // TODO: make this abstract
+        return true;
+    }
+
 /// performance and logging
     /**
      * Returns number of reads done by this database

diff --git a/lib/dml/mysqli_native_moodle_database.php b/lib/dml/mysqli_native_moodle_database.php
index 4f19b5c82273058c320332af0af1b7db16436e2b..59997bc562fa26d647004ff10c29441ee7962026 100644 (file)
--- a/lib/dml/mysqli_native_moodle_database.php
+++ b/lib/dml/mysqli_native_moodle_database.php
@@ -23,7 +23,7 @@ class mysqli_native_moodle_database extends moodle_database {
     /// TODO: Decide if this method should go to DDL instead of being here
     public function create_database($dbhost, $dbuser, $dbpass, $dbname) {
         ob_start();
-        $conn= new mysqli($dbhost, $dbuser, $dbpass); /// Connect without db
+        $conn = new mysqli($dbhost, $dbuser, $dbpass); /// Connect without db
         $dberr = ob_get_contents();
         ob_end_clean();
         $errorno = @$conn->connect_errno;
@@ -880,6 +880,51 @@ class mysqli_native_moodle_database extends moodle_database {
         return $positivematch ? 'REGEXP' : 'NOT REGEXP';
     }
 
+/// session locking
+    public function get_session_lock($name, $timeout) {
+        $fullname = $this->dbname.'-'.$this->prefix.'-'.$name;
+        $sql = "SELECT GET_LOCK(?,?)";
+        $params = array($fullname, $timeout);
+        $rawsql = $this->emulate_bound_params($sql, $params);
+
+        $this->query_start($sql, $params, SQL_QUERY_AUX);
+        $result = $this->mysqli->query($rawsql);
+        $this->query_end($result);
+
+        if ($result) {
+            $arr = $result->fetch_assoc();
+            $result->close();
+
+            if (reset($arr) == 1) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    public function release_session_lock($name) {
+        $fullname = $this->dbname.'-'.$this->prefix.'-'.$name;
+        $sql = "SELECT RELEASE_LOCK(?)";
+        $params = array($fullname);
+        $rawsql = $this->emulate_bound_params($sql, $params);
+
+        $this->query_start($sql, $params, SQL_QUERY_AUX);
+        $result = $this->mysqli->query($rawsql);
+        $this->query_end($result);
+
+        if ($result) {
+            $arr = $result->fetch_assoc();
+            $result->close();
+
+            if (reset($arr) == 1) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
 /// transactions
     /**
      * on DBs that support it, switch to transaction mode and begin a transaction

diff --git a/lib/sessionlib.php b/lib/sessionlib.php
index fa1f490b268aaa87e8dbc460fc0416e40ff7e832..1422e2916613dedcfc7efd36917fe359220991f1 100644 (file)
--- a/lib/sessionlib.php
+++ b/lib/sessionlib.php
@@ -311,7 +311,6 @@ class database_session extends session_stub {
     public function handler_read($sid) {
         global $CFG;
 
-        // TODO: implement normal locking (and later speculative locking)
         // TODO: implement timeout + auth plugin hook (see gc)
 
         if ($this->record and $this->record->sid != $sid) {
@@ -319,6 +318,11 @@ class database_session extends session_stub {
             return '';
         }
 
+        // lock session
+        while (!$this->database->get_session_lock($sid, 10)) {
+            sleep(1);
+        }
+
         try {
             if (!$record = $this->database->get_record('sessions', array('sid'=>$sid))) {
                 $record = new object();
@@ -367,6 +371,8 @@ class database_session extends session_stub {
             return true;
         }
 
+        $this->database->release_session_lock($this->record->sid);
+
         $this->record->sid          = $sid;                         // it might be regenerated
         $this->record->sessdata     = base64_encode($session_data); // there might be some binary mess :-(
         $this->record->sessdatahash = md5($this->record->sessdata);
@@ -391,6 +397,8 @@ class database_session extends session_stub {
             return true;
         }
 
+        $this->database->release_session_lock($this->record->sid);
+
         try {
             $this->database->delete_records('sessions', array('sid'=>$this->record->sid));
         } catch (dml_exception $ex) {
@@ -404,7 +412,7 @@ class database_session extends session_stub {
         $select = "timemodified + :maxlifetime < :now";
         $params = array('now'=>time(), 'maxlifetime'=>$maxlifetime);
 
-        // TODO: add auth plugin hook that would allow extennding of max lifetime
+        // TODO: add auth plugin hook that would allow extending of max lifetime
 
         try {
             $this->database->delete_records_select('sessions', $select, $params);