Merge branch 'w47_MDL-36211_m24_sesslocking' of git://github.com/skodak/moodle

This commit is contained in:
Dan Poltawski 2012-11-21 11:56:44 +08:00
commit 31112f3bab
8 changed files with 79 additions and 14 deletions

View File

@ -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

View File

@ -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;

View File

@ -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;

View File

@ -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')";

View File

@ -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;

View File

@ -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)";

View File

@ -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;

View File

@ -467,6 +467,9 @@ class database_session extends session_stub {
/** @var bool $failed session read/init failed, do not write back to DB */
protected $failed = false;
/** @var string hash of the session data content */
protected $lasthash = null;
public function __construct() {
global $DB;
$this->database = $DB;
@ -484,7 +487,7 @@ class database_session extends session_stub {
/**
* Check for existing session with id $sid
* @param unknown_type $sid
* @param string $sid
* @return boolean true if session found.
*/
public function session_exists($sid){
@ -572,7 +575,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 +594,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 +611,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('Cannot read session record');
$this->failed = true;
return '';
}
// verify timeout
if ($record->timemodified + $CFG->sessiontimeout < time()) {
$ignoretimeout = false;
@ -650,7 +668,13 @@ class database_session extends session_stub {
}
}
$data = is_null($record->sessdata) ? '' : base64_decode($record->sessdata);
if (is_null($record->sessdata)) {
$data = '';
$this->lasthash = sha1('');
} else {
$data = base64_decode($record->sessdata);
$this->lasthash = sha1($record->sessdata);
}
unset($record->sessdata); // conserve memory
$this->record = $record;
@ -688,16 +712,28 @@ class database_session extends session_stub {
}
if (isset($this->record->id)) {
$this->record->sessdata = base64_encode($session_data); // there might be some binary mess :-(
$data = base64_encode($session_data); // There might be some binary mess :-(
// Skip db update if nothing changed,
// do not update the timemodified each second.
$hash = sha1($data);
if ($this->lasthash === $hash
and $this->record->userid == $userid
and (time() - $this->record->timemodified < 20)
and $this->record->lastip == getremoteaddr()
) {
// No need to update anything!
return true;
}
$this->record->sessdata = $data;
$this->record->userid = $userid;
$this->record->timemodified = time();
$this->record->lastip = getremoteaddr();
// TODO: verify session changed before doing update,
// also make sure the timemodified field is changed only every 10s if nothing else changes MDL-20462
try {
$this->database->update_record_raw('sessions', $this->record);
$this->lasthash = $hash;
} catch (dml_exception $ex) {
if ($this->database->get_dbfamily() === 'mysql') {
try {
@ -725,7 +761,9 @@ class database_session extends session_stub {
$record->timecreated = $record->timemodified = time();
$record->firstip = $record->lastip = getremoteaddr();
$record->id = $this->database->insert_record_raw('sessions', $record);
$this->record = $record;
$this->record = $this->database->get_record('sessions', array('id'=>$record->id));
$this->lasthash = sha1($record->sessdata);
$this->database->get_session_lock($this->record->id, SESSION_ACQUIRE_LOCK_TIMEOUT);
} catch (Exception $ex) {
@ -759,6 +797,8 @@ class database_session extends session_stub {
$this->record = null;
}
$this->lasthash = null;
return true;
}