mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-17 22:45:54 +02:00
Code Issues Projects Releases Wiki Activity

Merge branch 'MDL-66935-lock-clashes' of https://github.com/brendanheywood/moodle

Browse Source
This commit is contained in:
Eloy Lafuente (stronk7) 2019-10-24 09:59:19 +02:00
commit 2ffdd72618
5 changed files with 86 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
lib/classes/lock/db_record_lock_factory.php
View File

@ -120,15 +120,17 @@ class db_record_lock_factory implements lock_factory {
$giveuptime = $now + $timeout;
$expires = $now + $maxlifetime;
if (!$this->db->record_exists('lock_db', array('resourcekey' => $resource))) {
$resourcekey = $this->type . '_' . $resource;
if (!$this->db->record_exists('lock_db', array('resourcekey' => $resourcekey))) {
$record = new \stdClass();
$record->resourcekey = $resource;
$record->resourcekey = $resourcekey;
$result = $this->db->insert_record('lock_db', $record);
}
$params = array('expires' => $expires,
'token' => $token,
'resourcekey' => $resource,
'resourcekey' => $resourcekey,
'now' => $now);
$sql = 'UPDATE {lock_db}
SET
@ -143,7 +145,7 @@ class db_record_lock_factory implements lock_factory {
$params['now'] = $now;
$this->db->execute($sql, $params);
$countparams = array('owner' => $token, 'resourcekey' => $resource);
$countparams = array('owner' => $token, 'resourcekey' => $resourcekey);
$result = $this->db->count_records('lock_db', $countparams);
$locked = $result === 1;
if (!$locked && $timeout > 0) {

35
lib/classes/lock/lock_config.php
View File

@ -38,18 +38,17 @@ defined('MOODLE_INTERNAL') || die();
class lock_config {
/**
* Get an instance of the currently configured locking subclass.
* Get the currently configured locking subclass.
*
* @param string $type - Unique namespace for the locks generated by this factory. e.g. core_cron
* @return \core\lock\lock_factory
* @return string class name
* @throws \coding_exception
*/
public static function get_lock_factory($type) {
public static function get_lock_factory_class(): string {
global $CFG, $DB;
$lockfactory = null;
if (during_initial_install()) {
$lockfactory = new \core\lock\installation_lock_factory($type);
$lockfactoryclass = '\core\lock\installation_lock_factory';
} else if (isset($CFG->lock_factory) && $CFG->lock_factory != 'auto') {
if (!class_exists($CFG->lock_factory)) {
// In this case I guess it is not safe to continue. Different cluster nodes could end up using different locking
@ -57,7 +56,6 @@ class lock_config {
throw new \coding_exception('Lock factory set in $CFG does not exist: ' . $CFG->lock_factory);
}
$lockfactoryclass = $CFG->lock_factory;
$lockfactory = new $lockfactoryclass($type);
} else {
$dbtype = clean_param($DB->get_dbfamily(), PARAM_ALPHA);
@ -66,14 +64,31 @@ class lock_config {
if (!class_exists($lockfactoryclass)) {
$lockfactoryclass = '\core\lock\file_lock_factory';
}
/* @var lock_factory $lockfactory */
$lockfactory = new $lockfactoryclass($type);
// Test if the auto chosen lock factory is available.
$lockfactory = new $lockfactoryclass('test');
if (!$lockfactory->is_available()) {
// Final fallback - DB row locking.
$lockfactory = new \core\lock\db_record_lock_factory($type);
$lockfactoryclass = '\core\lock\db_record_lock_factory';
}
}
return $lockfactoryclass;
}
/**
* Get an instance of the currently configured locking subclass.
*
* @param string $type - Unique namespace for the locks generated by this factory. e.g. core_cron
* @return \core\lock\lock_factory
* @throws \coding_exception
*/
public static function get_lock_factory(string $type): \core\lock\lock_factory {
$lockfactoryclass = self::get_lock_factory_class();
$lockfactory = new $lockfactoryclass($type);
if (!$lockfactory->is_available()) {
throw new \coding_exception("Lock factory class $lockfactoryclass is not available.");
}
return $lockfactory;
}

2
lib/classes/lock/postgres_lock_factory.php
View File

@ -178,7 +178,7 @@ class postgres_lock_factory implements lock_factory {
public function get_lock($resource, $timeout, $maxlifetime = 86400) {
$giveuptime = time() + $timeout;
$token = $this->get_index_from_key($resource);
$token = $this->get_index_from_key($this->type . '_' . $resource);
$params = array('locktype' => $this->dblockid,
'token' => $token);

38
lib/tests/lock_config_test.php
View File

@ -46,31 +46,57 @@ class lock_config_testcase extends advanced_testcase {
if (isset($CFG->lock_factory)) {
$original = $CFG->lock_factory;
}
$originalfilelocking = null;
if (isset($CFG->preventfilelocking)) {
$originalfilelocking = $CFG->preventfilelocking;
}
// Test no configuration.
unset($CFG->lock_factory);
$CFG->preventfilelocking = 0;
$factory = \core\lock\lock_config::get_lock_factory('cache');
$this->assertNotEmpty($factory, 'Get a default factory with no configuration');
$CFG->lock_factory = '\core\lock\file_lock_factory';
// Test explicit broken lock.
$CFG->lock_factory = '\core\lock\not_a_lock_factory';
try {
$factory = \core\lock\lock_config::get_lock_factory('cache');
$this->fail('Exception expected');
} catch (moodle_exception $ex) {
$this->assertInstanceOf('coding_exception', $ex);
}
// Test explicit file locks.
$CFG->lock_factory = '\core\lock\file_lock_factory';
$factory = \core\lock\lock_config::get_lock_factory('cache');
$this->assertTrue($factory instanceof \core\lock\file_lock_factory,
'Get a default factory with a set configuration');
'Get an explicit file lock factory');
// Test explicit file locks but with file locks prevented.
$CFG->preventfilelocking = 1;
try {
$factory = \core\lock\lock_config::get_lock_factory('cache');
$this->fail('Exception expected');
} catch (moodle_exception $ex) {
$this->assertInstanceOf('coding_exception', $ex);
}
// Test explicit db locks.
$CFG->lock_factory = '\core\lock\db_record_lock_factory';
$factory = \core\lock\lock_config::get_lock_factory('cache');
$this->assertTrue($factory instanceof \core\lock\db_record_lock_factory,
'Get a default factory with a changed configuration');
'Get an explicit db record lock factory');
if ($original) {
$CFG->lock_factory = $original;
} else {
unset($CFG->lock_factory);
}
if ($originalfilelocking) {
$CFG->preventfilelocking = $originalfilelocking;
} else {
unset($CFG->preventfilelocking);
}
}
}

33
lib/tests/lock_test.php
View File

@ -44,11 +44,26 @@ class lock_testcase extends advanced_testcase {
}
/**
* Run a suite of tests on a lock factory.
* @param \core\lock\lock_factory $lockfactory - A lock factory to test
* Run a suite of tests on a lock factory class.
*
* @param class $lockfactoryclass - A lock factory class to test
*/
protected function run_on_lock_factory(\core\lock\lock_factory $lockfactory) {
protected function run_on_lock_factory($lockfactoryclass) {
$modassignfactory = new $lockfactoryclass('mod_assign');
$tooltaskfactory = new $lockfactoryclass('tool_task');
// Test for lock clashes between lock stores.
$assignlock = $modassignfactory->get_lock('abc', 0);
$this->assertNotEmpty($assignlock, 'Get a lock "abc" from store "mod_assign"');
$tasklock = $tooltaskfactory->get_lock('abc', 0);
$this->assertNotEmpty($tasklock, 'Get a lock "abc" from store "tool_task"');
$assignlock->release();
$tasklock->release();
$lockfactory = new $lockfactoryclass('default');
if ($lockfactory->is_available()) {
// This should work.
$lock1 = $lockfactory->get_lock('abc', 2);
@ -111,20 +126,16 @@ class lock_testcase extends advanced_testcase {
}
/**
* Tests the testable lock factories.
* Tests the testable lock factories classes.
* @return void
*/
public function test_locks() {
// Run the suite on the current configured default (may be non-core).
$defaultfactory = \core\lock\lock_config::get_lock_factory('default');
$this->run_on_lock_factory($defaultfactory);
$this->run_on_lock_factory(\core\lock\lock_config::get_lock_factory_class());
// Manually create the core no-configuration factories.
$dblockfactory = new \core\lock\db_record_lock_factory('test');
$this->run_on_lock_factory($dblockfactory);
$filelockfactory = new \core\lock\file_lock_factory('test');
$this->run_on_lock_factory($filelockfactory);
$this->run_on_lock_factory(\core\lock\db_record_lock_factory::class);
$this->run_on_lock_factory(\core\lock\file_lock_factory::class);
}