MDL-36211 do not lock sessions for guests and not-logged-in users

config-dist.php

@@ -246,6 +246,11 @@ $CFG->admin = 'admin';
 // logs in. The site front page will always show the same (logged-out) view.
 //     $CFG->disablemycourses = true;
 //
+// By default all user sessions should be using locking, uncomment
+// the following setting to prevent locking for guests and not-logged-in
+// accounts. This may improve performance significantly.
+//     $CFG->sessionlockloggedinonly = 1;
+//
 // If this setting is set to true, then Moodle will track the IP of the
 // current user to make sure it hasn't changed during a session.  This
 // will prevent the possibility of sessions being hijacked via XSS, but it

lib/dml/moodle_database.php

@@ -341,11 +341,11 @@ abstract class moodle_database {
             }
             $this->force_transaction_rollback();
         }
-        if ($this->used_for_db_sessions) {
-            // this is needed because we need to save session to db before closing it
-            session_get_instance()->write_close();
-            $this->used_for_db_sessions = false;
-        }
+        // Always terminate sessions here to make it consistent,
+        // this is needed because we need to save session to db before closing it.
+        session_get_instance()->write_close();
+        $this->used_for_db_sessions = false;
+
         if ($this->temptables) {
             $this->temptables->dispose();
             $this->temptables = null;

lib/dml/mssql_native_moodle_database.php

@@ -1296,6 +1296,10 @@ s only returning name of SQL substring function, it now requires all parameters.
         if (!$this->session_lock_supported()) {
             return;
         }
+        if (!$this->used_for_db_sessions) {
+            return;
+        }
+
         parent::release_session_lock($rowid);
 
         $fullname = $this->dbname.'-'.$this->prefix.'-session-'.$rowid;

lib/dml/mysqli_native_moodle_database.php

@@ -1451,6 +1451,10 @@ class mysqli_native_moodle_database extends moodle_database {
     }
 
     public function release_session_lock($rowid) {
+        if (!$this->used_for_db_sessions) {
+            return;
+        }
+
         parent::release_session_lock($rowid);
         $fullname = $this->dbname.'-'.$this->prefix.'-session-'.$rowid;
         $sql = "SELECT RELEASE_LOCK('$fullname')";

lib/dml/oci_native_moodle_database.php

@@ -1649,6 +1649,10 @@ class oci_native_moodle_database extends moodle_database {
         if (!$this->session_lock_supported()) {
             return;
         }
+        if (!$this->used_for_db_sessions) {
+            return;
+        }
+
         parent::release_session_lock($rowid);
 
         $fullname = $this->dbname.'-'.$this->prefix.'-session-'.$rowid;

lib/dml/pgsql_native_moodle_database.php

@@ -1289,6 +1289,10 @@ class pgsql_native_moodle_database extends moodle_database {
         if (!$this->session_lock_supported()) {
             return;
         }
+        if (!$this->used_for_db_sessions) {
+            return;
+        }
+
         parent::release_session_lock($rowid);
 
         $sql = "SELECT pg_advisory_unlock($rowid)";

lib/dml/sqlsrv_native_moodle_database.php

@@ -1347,6 +1347,10 @@ class sqlsrv_native_moodle_database extends moodle_database {
         if (!$this->session_lock_supported()) {
             return;
         }
+        if (!$this->used_for_db_sessions) {
+            return;
+        }
+
         parent::release_session_lock($rowid);
 
         $fullname = $this->dbname.'-'.$this->prefix.'-session-'.$rowid;

lib/sessionlib.php

@@ -572,7 +572,8 @@ class database_session extends session_stub {
         }
 
         try {
-            if (!$record = $this->database->get_record('sessions', array('sid'=>$sid))) {
+            // Do not fetch full record yet, wait until it is locked.
+            if (!$record = $this->database->get_record('sessions', array('sid'=>$sid), 'id, userid')) {
                 $record = new stdClass();
                 $record->state        = 0;
                 $record->sid          = $sid;
@@ -590,7 +591,14 @@ class database_session extends session_stub {
         }
 
         try {
-            $this->database->get_session_lock($record->id, SESSION_ACQUIRE_LOCK_TIMEOUT);
+            if (!empty($CFG->sessionlockloggedinonly) and (isguestuser($record->userid) or empty($record->userid))) {
+                // No session locking for guests and not-logged-in users,
+                // these users mostly read stuff, there should not be any major
+                // session race conditions. Hopefully they do not access other
+                // pages while being logged-in.
+            } else {
+                $this->database->get_session_lock($record->id, SESSION_ACQUIRE_LOCK_TIMEOUT);
+            }
         } catch (Exception $ex) {
             // This is a fatal error, better inform users.
             // It should not happen very often - all pages that need long time to execute
@@ -600,6 +608,13 @@ class database_session extends session_stub {
             throw $ex;
         }
 
+        // Finally read the full session data because we know we have the lock now.
+        if (!$record = $this->database->get_record('sessions', array('id'=>$record->id))) {
+            error_log('Can read session record');
+            $this->failed = true;
+            return '';
+        }
+
         // verify timeout
         if ($record->timemodified + $CFG->sessiontimeout < time()) {
             $ignoretimeout = false;