Skip to content
Browse files

MDL-30026 improve session lock acquire timeouts and other minor cleanup

This is partially based on original patch by Tony Levi.
  • Loading branch information...
1 parent 8e35d0a commit a6ecee4ca09cd51215941115880aac3afb99ff32 @skodak skodak committed Nov 6, 2011
View
1 lang/en/error.php
@@ -433,6 +433,7 @@
$string['sectionnotexist'] = 'This section does not exist';
$string['sendmessage'] = 'Send message';
$string['servicedonotexist'] = 'The service does not exist';
+$string['sessionwaiterr'] = 'Timed out while waiting for session lock.<br />Wait for your current requests to finish and try again later.';
$string['sessioncookiesdisable'] = 'Incorrect use of require_key_login() - session cookies must be disabled!';
$string['sessiondiskfull'] = 'The session partition is full. It is not possible to login at this time.<br /><br />Please notify server administrator.';
$string['sessionerroruser'] = 'Your session has timed out. Please login again.';
View
3 lib/dml/moodle_database.php
@@ -2204,9 +2204,10 @@ public function session_lock_supported() {
/**
* Obtain session lock
* @param int $rowid id of the row with session record
+ * @param int $timeout max allowed time to wait for the lock in seconds
* @return bool success
*/
- public function get_session_lock($rowid) {
+ public function get_session_lock($rowid, $timeout) {
$this->used_for_db_sessions = true;
}
View
21 lib/dml/mssql_native_moodle_database.php
@@ -1227,18 +1227,33 @@ public function session_lock_supported() {
return true;
}
- public function get_session_lock($rowid) {
+ /**
+ * Obtain session lock
+ * @param int $rowid id of the row with session record
+ * @param int $timeout max allowed time to wait for the lock in seconds
+ * @return bool success
+ */
+ public function get_session_lock($rowid, $timeout) {
if (!$this->session_lock_supported()) {
return;
}
- parent::get_session_lock($rowid);
+ parent::get_session_lock($rowid, $timeout);
+
+ $timeoutmilli = $timeout * 1000;
$fullname = $this->dbname.'-'.$this->prefix.'-session-'.$rowid;
- $sql = "sp_getapplock '$fullname', 'Exclusive', 'Session', 120000";
+ $sql = "sp_getapplock '$fullname', 'Exclusive', 'Session', $timeoutmilli";
$this->query_start($sql, null, SQL_QUERY_AUX);
$result = mssql_query($sql, $this->mssql);
$this->query_end($result);
+ if ($result) {
+ $row = mssql_fetch_row($result);
+ if ($row[0] < 0) {
+ throw new dml_sessionwait_exception();
+ }
+ }
+
$this->free_result($result);
}
View
16 lib/dml/mysqli_native_moodle_database.php
@@ -1203,10 +1203,17 @@ public function session_lock_supported() {
return true;
}
- public function get_session_lock($rowid) {
- parent::get_session_lock($rowid);
+ /**
+ * Obtain session lock
+ * @param int $rowid id of the row with session record
+ * @param int $timeout max allowed time to wait for the lock in seconds
+ * @return bool success
+ */
+ public function get_session_lock($rowid, $timeout) {
+ parent::get_session_lock($rowid, $timeout);
+
$fullname = $this->dbname.'-'.$this->prefix.'-session-'.$rowid;
- $sql = "SELECT GET_LOCK('$fullname',120)";
+ $sql = "SELECT GET_LOCK('$fullname', $timeout)";
$this->query_start($sql, null, SQL_QUERY_AUX);
$result = $this->mysqli->query($sql);
$this->query_end($result);
@@ -1218,8 +1225,7 @@ public function get_session_lock($rowid) {
if (reset($arr) == 1) {
return;
} else {
- // try again!
- $this->get_session_lock($rowid);
+ throw new dml_sessionwait_exception();
}
}
}
View
19 lib/dml/oci_native_moodle_database.php
@@ -1607,21 +1607,34 @@ public function session_lock_supported() {
return $this->dblocks_supported;
}
- public function get_session_lock($rowid) {
+ /**
+ * Obtain session lock
+ * @param int $rowid id of the row with session record
+ * @param int $timeout max allowed time to wait for the lock in seconds
+ * @return bool success
+ */
+ public function get_session_lock($rowid, $timeout) {
if (!$this->session_lock_supported()) {
return;
}
- parent::get_session_lock($rowid);
+ parent::get_session_lock($rowid, $timeout);
$fullname = $this->dbname.'-'.$this->prefix.'-session-'.$rowid;
$sql = 'SELECT MOODLE_LOCKS.GET_LOCK(:lockname, :locktimeout) FROM DUAL';
- $params = array('lockname' => $fullname , 'locktimeout' => 120);
+ $params = array('lockname' => $fullname , 'locktimeout' => $timeout);
$this->query_start($sql, $params, SQL_QUERY_AUX);
$stmt = $this->parse_query($sql);
$this->bind_params($stmt, $params);
+ $start = time();
$result = oci_execute($stmt, $this->commit_status);
+ $end = time();
$this->query_end($result, $stmt);
oci_free_statement($stmt);
+
+ if ($end - $start >= $timeout) {
+ //TODO: there has to be a better way to find out if lock obtained
+ throw new dml_sessionwait_exception();
+ }
}
public function release_session_lock($rowid) {
View
41 lib/dml/pgsql_native_moodle_database.php
@@ -1153,17 +1153,54 @@ public function session_lock_supported() {
return true;
}
- public function get_session_lock($rowid) {
+ /**
+ * Obtain session lock
+ * @param int $rowid id of the row with session record
+ * @param int $timeout max allowed time to wait for the lock in seconds
+ * @return bool success
+ */
+ public function get_session_lock($rowid, $timeout) {
// NOTE: there is a potential locking problem for database running
// multiple instances of moodle, we could try to use pg_advisory_lock(int, int),
// luckily there is not a big chance that they would collide
if (!$this->session_lock_supported()) {
return;
}
- parent::get_session_lock($rowid);
+ parent::get_session_lock($rowid, $timeout);
+
+ $timeoutmilli = $timeout * 1000;
+
+ $sql = "SET statement_timeout TO $timeoutmilli";
+ $this->query_start($sql, null, SQL_QUERY_AUX);
+ $result = pg_query($this->pgsql, $sql);
+ $this->query_end($result);
+
+ if ($result) {
+ pg_free_result($result);
+ }
+
$sql = "SELECT pg_advisory_lock($rowid)";
$this->query_start($sql, null, SQL_QUERY_AUX);
+ $start = time();
+ $result = pg_query($this->pgsql, $sql);
+ $end = time();
+ try {
+ $this->query_end($result);
+ } catch (dml_exception $ex) {
+ if ($end - $start >= $timeout) {
+ throw new dml_sessionwait_exception();
+ } else {
+ throw $ex;
+ }
+ }
+
+ if ($result) {
+ pg_free_result($result);
+ }
+
+ $sql = "SET statement_timeout TO DEFAULT";
+ $this->query_start($sql, null, SQL_QUERY_AUX);
$result = pg_query($this->pgsql, $sql);
$this->query_end($result);
View
22 lib/dml/sqlsrv_native_moodle_database.php
@@ -1286,17 +1286,33 @@ public function session_lock_supported() {
return true;
}
- public function get_session_lock($rowid) {
+ /**
+ * Obtain session lock
+ * @param int $rowid id of the row with session record
+ * @param int $timeout max allowed time to wait for the lock in seconds
+ * @return bool success
+ */
+ public function get_session_lock($rowid, $timeout) {
if (!$this->session_lock_supported()) {
return;
}
- parent::get_session_lock($rowid);
+ parent::get_session_lock($rowid, $timeout);
+
+ $timeoutmilli = $timeout * 1000;
$fullname = $this->dbname.'-'.$this->prefix.'-session-'.$rowid;
- $sql = "sp_getapplock '$fullname', 'Exclusive', 'Session', 120000";
+ $sql = "sp_getapplock '$fullname', 'Exclusive', 'Session', $timeoutmilli";
$this->query_start($sql, null, SQL_QUERY_AUX);
$result = sqlsrv_query($this->sqlsrv, $sql);
$this->query_end($result);
+
+ if ($result) {
+ $row = sqlsrv_fetch_array($result);
+ if ($row[0] < 0) {
+ throw new dml_sessionwait_exception();
+ }
+ }
+
$this->free_result($result);
}
View
12 lib/dmllib.php
@@ -79,6 +79,18 @@ function __construct($error) {
}
/**
+ * DML db session wait exception - triggered when session lock request times out.
+ */
+class dml_sessionwait_exception extends dml_exception {
+ /**
+ * Constructor
+ */
+ function __construct() {
+ parent::__construct('sessionwaiterr');
+ }
+}
+
+/**
* DML read exception - triggered by SQL syntax errors, missing tables, etc.
*/
class dml_read_exception extends dml_exception {
View
254 lib/sessionlib.php
@@ -1,5 +1,4 @@
<?php
-
// This file is part of Moodle - http://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
@@ -25,6 +24,14 @@
defined('MOODLE_INTERNAL') || die();
+if (!defined('SESSION_ACQUIRE_LOCK_TIMEOUT')) {
+ /**
+ * How much time to wait for session lock before displaying error (in seconds),
+ * 3 minutes by default should be a reasonable time before telling users to wait and refresh browser.
+ */
+ define('SESSION_ACQUIRE_LOCK_TIMEOUT', 60*2);
+}
+
/**
* Factory method returning moodle_session object.
* @return moodle_session
@@ -39,27 +46,34 @@ function session_get_instance() {
$CFG->sessiontimeout = 7200;
}
- if (defined('SESSION_CUSTOM_CLASS')) {
- // this is a hook for webservices, key based login, etc.
- if (defined('SESSION_CUSTOM_FILE')) {
- require_once($CFG->dirroot.SESSION_CUSTOM_FILE);
- }
- $session_class = SESSION_CUSTOM_CLASS;
- $session = new $session_class();
+ try {
+ if (defined('SESSION_CUSTOM_CLASS')) {
+ // this is a hook for webservices, key based login, etc.
+ if (defined('SESSION_CUSTOM_FILE')) {
+ require_once($CFG->dirroot.SESSION_CUSTOM_FILE);
+ }
+ $session_class = SESSION_CUSTOM_CLASS;
+ $session = new $session_class();
- } else if ((!isset($CFG->dbsessions) or $CFG->dbsessions) and $DB->session_lock_supported()) {
- // default recommended session type
- $session = new database_session();
+ } else if ((!isset($CFG->dbsessions) or $CFG->dbsessions) and $DB->session_lock_supported()) {
+ // default recommended session type
+ $session = new database_session();
- } else {
- // legacy limited file based storage - some features and auth plugins will not work, sorry
- $session = new legacy_file_session();
+ } else {
+ // legacy limited file based storage - some features and auth plugins will not work, sorry
+ $session = new legacy_file_session();
+ }
+ } catch (Exception $ex) {
+ // prevent repeated inits
+ $session = new emergency_session();
+ throw $ex;
}
}
return $session;
}
+
/**
* Moodle session abstraction
*
@@ -90,6 +104,53 @@ public function write_close();
public function session_exists($sid);
}
+
+/**
+ * Fallback session handler when standard session init fails.
+ * This prevents repeated attempts to init faulty handler.
+ *
+ * @package core
+ * @subpackage session
+ * @copyright 2011 Petr Skoda {@link http://skodak.org}
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class emergency_session implements moodle_session {
+
+ public function __construct() {
+ // session not used at all
+ $_SESSION = array();
+ $_SESSION['SESSION'] = new stdClass();
+ $_SESSION['USER'] = new stdClass();
+ }
+
+ /**
+ * Terminate current session
+ * @return void
+ */
+ public function terminate_current() {
+ return;
+ }
+
+ /**
+ * No more changes in session expected.
+ * Unblocks the sessions, other scripts may start executing in parallel.
+ * @return void
+ */
+ public function write_close() {
+ return;
+ }
+
+ /**
+ * Check for existing session with id $sid
+ * @param unknown_type $sid
+ * @return boolean true if session found.
+ */
+ public function session_exists($sid) {
+ return false;
+ }
+}
+
+
/**
* Class handling all session and cookies related stuff.
*
@@ -142,7 +203,8 @@ public function __construct() {
}
/**
- * Terminates active moodle session
+ * Terminate current session
+ * @return void
*/
public function terminate_current() {
global $CFG, $SESSION, $USER, $DB;
@@ -305,11 +367,12 @@ protected function prepare_cookies() {
}
/**
- * Inits session storage.
+ * Init session storage.
*/
protected abstract function init_session_storage();
}
+
/**
* Legacy moodle sessions stored in files, not recommended any more.
*
@@ -319,6 +382,9 @@ protected function prepare_cookies() {
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class legacy_file_session extends session_stub {
+ /**
+ * Init session storage.
+ */
protected function init_session_storage() {
global $CFG;
@@ -358,6 +424,7 @@ public function session_exists($sid){
}
}
+
/**
* Recommended moodle session storage.
*
@@ -367,9 +434,15 @@ public function session_exists($sid){
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class database_session extends session_stub {
+ /** @var stdClass $record session record */
protected $record = null;
+
+ /** @var moodle_database $database session database */
protected $database = null;
+ /** @var bool $failed session read/init failed, do not write back to DB */
+ protected $failed = false;
+
public function __construct() {
global $DB;
$this->database = $DB;
@@ -385,6 +458,11 @@ public function __construct() {
}
}
+ /**
+ * Check for existing session with id $sid
+ * @param unknown_type $sid
+ * @return boolean true if session found.
+ */
public function session_exists($sid){
global $CFG;
try {
@@ -398,6 +476,9 @@ public function session_exists($sid){
}
}
+ /**
+ * Init session storage.
+ */
protected function init_session_storage() {
global $CFG;
@@ -417,31 +498,57 @@ protected function init_session_storage() {
}
}
+ /**
+ * Open session handler
+ *
+ * {@see http://php.net/manual/en/function.session-set-save-handler.php}
+ *
+ * @param string $save_path
+ * @param string $session_name
+ * @return bool success
+ */
public function handler_open($save_path, $session_name) {
return true;
}
+ /**
+ * Close session handler
+ *
+ * {@see http://php.net/manual/en/function.session-set-save-handler.php}
+ *
+ * @return bool success
+ */
public function handler_close() {
if (isset($this->record->id)) {
- $this->database->release_session_lock($this->record->id);
+ try {
+ $this->database->release_session_lock($this->record->id);
+ } catch (Exception $ex) {
+ // ignore any problems
+ }
}
$this->record = null;
return true;
}
+ /**
+ * Read session handler
+ *
+ * {@see http://php.net/manual/en/function.session-set-save-handler.php}
+ *
+ * @param string $sid
+ * @return string
+ */
public function handler_read($sid) {
global $CFG;
if ($this->record and $this->record->sid != $sid) {
error_log('Weird error reading database session - mismatched sid');
+ $this->failed = true;
return '';
}
try {
- if ($record = $this->database->get_record('sessions', array('sid'=>$sid))) {
- $this->database->get_session_lock($record->id);
-
- } else {
+ if (!$record = $this->database->get_record('sessions', array('sid'=>$sid))) {
$record = new stdClass();
$record->state = 0;
$record->sid = $sid;
@@ -450,14 +557,25 @@ public function handler_read($sid) {
$record->timecreated = $record->timemodified = time();
$record->firstip = $record->lastip = getremoteaddr();
$record->id = $this->database->insert_record_raw('sessions', $record);
-
- $this->database->get_session_lock($record->id);
}
- } catch (dml_exception $ex) {
+ } catch (Exception $ex) {
+ // do not rethrow exceptions here, we need this to work somehow before 1.9.x upgrade and during install
error_log('Can not read or insert database sessions');
+ $this->failed = true;
return '';
}
+ try {
+ $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
+ // should close session soon after access control checks
+ error_log('Can not obtain session lock');
+ $this->failed = true;
+ throw $ex;
+ }
+
// verify timeout
if ($record->timemodified + $CFG->sessiontimeout < time()) {
$ignoretimeout = false;
@@ -480,9 +598,11 @@ public function handler_read($sid) {
$record->timemodified = time();
try {
$this->database->update_record('sessions', $record);
- } catch (dml_exception $ex) {
+ } catch (Exception $ex) {
+ // very unlikely error
error_log('Can not refresh database session');
- return '';
+ $this->failed = true;
+ throw $ex;
}
} else {
//time out session
@@ -493,9 +613,11 @@ public function handler_read($sid) {
$record->firstip = $record->lastip = getremoteaddr();
try {
$this->database->update_record('sessions', $record);
- } catch (dml_exception $ex) {
+ } catch (Exception $ex) {
+ // very unlikely error
error_log('Can not time out database session');
- return '';
+ $this->failed = true;
+ throw $ex;
}
}
}
@@ -508,11 +630,28 @@ public function handler_read($sid) {
return $data;
}
+ /**
+ * Write session handler.
+ *
+ * {@see http://php.net/manual/en/function.session-set-save-handler.php}
+ *
+ * NOTE: Do not write to output or throw any exceptions!
+ * Hopefully the next page is going to display nice error or it recovers...
+ *
+ * @param string $sid
+ * @param string $session_data
+ * @return bool success
+ */
public function handler_write($sid, $session_data) {
global $USER;
// TODO: MDL-20625 we need to rollback all active transactions and log error if any open needed
+ if ($this->failed) {
+ // do not write anything back - we failed to start the session properly
+ return false;
+ }
+
$userid = 0;
if (!empty($USER->realuser)) {
$userid = $USER->realuser;
@@ -539,53 +678,80 @@ public function handler_write($sid, $session_data) {
try {
$this->database->set_field('sessions', 'state', 9, array('id'=>$this->record->id));
} catch (Exception $ignored) {
-
}
error_log('Can not write database session - please verify max_allowed_packet is at least 4M!');
} else {
error_log('Can not write database session');
}
+ return false;
+ } catch (Exception $ex) {
+ error_log('Can not write database session');
+ return false;
}
} else {
- // session already destroyed
- $record = new stdClass();
- $record->state = 0;
- $record->sid = $sid;
- $record->sessdata = base64_encode($session_data); // there might be some binary mess :-(
- $record->userid = $userid;
- $record->timecreated = $record->timemodified = time();
- $record->firstip = $record->lastip = getremoteaddr();
- $record->id = $this->database->insert_record_raw('sessions', $record);
- $this->record = $record;
-
+ // fresh new session
try {
- $this->database->get_session_lock($this->record->id);
- } catch (dml_exception $ex) {
- error_log('Can not write new database session');
+ $record = new stdClass();
+ $record->state = 0;
+ $record->sid = $sid;
+ $record->sessdata = base64_encode($session_data); // there might be some binary mess :-(
+ $record->userid = $userid;
+ $record->timecreated = $record->timemodified = time();
+ $record->firstip = $record->lastip = getremoteaddr();
+ $record->id = $this->database->insert_record_raw('sessions', $record);
+ $this->record = $record;
+
+ $this->database->get_session_lock($this->record->id, SESSION_ACQUIRE_LOCK_TIMEOUT);
+ } catch (Exception $ex) {
+ // this should not happen
+ error_log('Can not write new database session or acquire session lock');
+ $this->failed = true;
+ return false;
}
}
return true;
}
+ /**
+ * Destroy session handler
+ *
+ * {@see http://php.net/manual/en/function.session-set-save-handler.php}
+ *
+ * @param string $sid
+ * @return bool success
+ */
public function handler_destroy($sid) {
session_kill($sid);
if (isset($this->record->id) and $this->record->sid === $sid) {
- $this->database->release_session_lock($this->record->id);
+ try {
+ $this->database->release_session_lock($this->record->id);
+ } catch (Exception $ex) {
+ // ignore problems
+ }
$this->record = null;
}
return true;
}
+ /**
+ * GC session handler
+ *
+ * {@see http://php.net/manual/en/function.session-set-save-handler.php}
+ *
+ * @param int $ignored_maxlifetime moodle uses special timeout rules
+ * @return bool success
+ */
public function handler_gc($ignored_maxlifetime) {
session_gc();
return true;
}
}
+
/**
* returns true if legacy session used.
* @return bool true if legacy(==file) based session used

0 comments on commit a6ecee4

Please sign in to comment.
Something went wrong with that request. Please try again.