diff --git a/config-dist.php b/config-dist.php index 2e38635377f..cb1aa0d25d5 100644 --- a/config-dist.php +++ b/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 diff --git a/lib/dml/moodle_database.php b/lib/dml/moodle_database.php index c81dcf633c2..2983d155f41 100644 --- a/lib/dml/moodle_database.php +++ b/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; diff --git a/lib/dml/mssql_native_moodle_database.php b/lib/dml/mssql_native_moodle_database.php index 323a2fe3c57..11045181596 100644 --- a/lib/dml/mssql_native_moodle_database.php +++ b/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; diff --git a/lib/dml/mysqli_native_moodle_database.php b/lib/dml/mysqli_native_moodle_database.php index 44f3a92562e..fa823e869ce 100644 --- a/lib/dml/mysqli_native_moodle_database.php +++ b/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')"; diff --git a/lib/dml/oci_native_moodle_database.php b/lib/dml/oci_native_moodle_database.php index b62ac9baddc..eaafb543439 100644 --- a/lib/dml/oci_native_moodle_database.php +++ b/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; diff --git a/lib/dml/pgsql_native_moodle_database.php b/lib/dml/pgsql_native_moodle_database.php index ab3650fcaf3..9c43a16e54b 100644 --- a/lib/dml/pgsql_native_moodle_database.php +++ b/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)"; diff --git a/lib/dml/sqlsrv_native_moodle_database.php b/lib/dml/sqlsrv_native_moodle_database.php index 168c68234c0..30ceb2c8368 100644 --- a/lib/dml/sqlsrv_native_moodle_database.php +++ b/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; diff --git a/lib/sessionlib.php b/lib/sessionlib.php index 1e7e927d73d..1e38e6bf6f9 100644 --- a/lib/sessionlib.php +++ b/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;