mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-03-14 20:50:21 +01:00
Code Issues Projects Releases Wiki Activity

MDL-79349 core_cache: Lock on multi-layer cache can behave incorrectly

Browse Source
This commit is contained in:
sam marshall 2023-09-13 16:23:48 +01:00
parent 5058c46813
commit f6f0787fc8
2 changed files with 104 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
cache/classes/loaders.php vendored
View File

@ -1676,24 +1676,34 @@ class cache_application extends cache implements cache_loader_with_locking {
* @return bool Returns true if the lock could be acquired, false otherwise.
*/
public function acquire_lock($key) {
global $CFG;
$releaseparent = false;
if ($this->get_loader() !== false) {
$this->get_loader()->acquire_lock($key);
if (!$this->get_loader()->acquire_lock($key)) {
return false;
}
// We need to release this lock later if the lock is not successful.
$releaseparent = true;
}
$key = cache_helper::hash_key($key, $this->get_definition());
$hashedkey = cache_helper::hash_key($key, $this->get_definition());
$before = microtime(true);
if ($this->nativelocking) {
$lock = $this->get_store()->acquire_lock($key, $this->get_identifier());
$lock = $this->get_store()->acquire_lock($hashedkey, $this->get_identifier());
} else {
$this->ensure_cachelock_available();
$lock = $this->cachelockinstance->lock($key, $this->get_identifier());
$lock = $this->cachelockinstance->lock($hashedkey, $this->get_identifier());
}
$after = microtime(true);
if ($lock) {
$this->locks[$key] = $lock;
$this->locks[$hashedkey] = $lock;
if ((defined('MDL_PERF') && MDL_PERF) || $this->perfdebug) {
\core\lock\timing_wrapper_lock_factory::record_lock_data($after, $before,
$this->get_definition()->get_id(), $key, $lock, $this->get_identifier() . $key);
$this->get_definition()->get_id(), $hashedkey, $lock, $this->get_identifier() . $hashedkey);
}
} else {
// If we successfully got the parent lock, but are now failing to get this lock, then we should release
// the parent one.
if ($releaseparent) {
$this->get_loader()->release_lock($key);
}
}
return $lock;

87
cache/tests/cache_test.php vendored
View File

@ -2274,6 +2274,93 @@ class cache_test extends \advanced_testcase {
$this->assertEquals(['x' => 'X', 'y' => 'Y', 'z' => 'Z'], $cache->get_many(['x', 'y', 'z']));
}
/**
* Tests that locking fails correctly when either layer of a 2-layer cache has a lock already.
*
* @covers \cache_loader
*/
public function test_application_locking_multiple_layers_failures(): void {
$instance = cache_config_testing::instance(true);
$instance->phpunit_add_definition('phpunit/test_application_locking', array(
'mode' => cache_store::MODE_APPLICATION,
'component' => 'phpunit',
'area' => 'test_application_locking',
'staticacceleration' => true,
'staticaccelerationsize' => 1,
'requirelockingbeforewrite' => true
), false);
$instance->phpunit_add_file_store('phpunittest1');
$instance->phpunit_add_file_store('phpunittest2');
$instance->phpunit_add_definition_mapping('phpunit/test_application_locking', 'phpunittest1', 1);
$instance->phpunit_add_definition_mapping('phpunit/test_application_locking', 'phpunittest2', 2);
$cache = cache::make('phpunit', 'test_application_locking');
// We need to get the individual stores so as to set up the right behaviour here.
$ref = new \ReflectionClass('\cache');
$definitionprop = $ref->getProperty('definition');
$definitionprop->setAccessible(true);
$storeprop = $ref->getProperty('store');
$storeprop->setAccessible(true);
$loaderprop = $ref->getProperty('loader');
$loaderprop->setAccessible(true);
$definition = $definitionprop->getValue($cache);
$localstore = $storeprop->getValue($cache);
$sharedcache = $loaderprop->getValue($cache);
$sharedstore = $storeprop->getValue($sharedcache);
// Set the lock waiting time to 1 second so it doesn't take forever to run the test.
$ref = new \ReflectionClass('\cachestore_file');
$lockwaitprop = $ref->getProperty('lockwait');
$lockwaitprop->setAccessible(true);
$lockwaitprop->setValue($localstore, 1);
$lockwaitprop->setValue($sharedstore, 1);
// Get key details and the cache identifier.
$hashedkey = cache_helper::hash_key('apple', $definition);
$localidentifier = $cache->get_identifier();
$sharedidentifier = $sharedcache->get_identifier();
// 1. Local cache is not locked but parent cache is locked.
$sharedstore->acquire_lock($hashedkey, 'somebodyelse');
try {
$this->assertFalse($cache->acquire_lock('apple'));
// Neither store is locked by us, shared store still locked.
$this->assertFalse((bool)$localstore->check_lock_state($hashedkey, $localidentifier));
$this->assertFalse((bool)$sharedstore->check_lock_state($hashedkey, $sharedidentifier));
$this->assertTrue((bool)$sharedstore->check_lock_state($hashedkey, 'somebodyelse'));
} finally {
$sharedstore->release_lock($hashedkey, 'somebodyelse');
}
// 2. Local cache is locked, parent cache is not locked.
$localstore->acquire_lock($hashedkey, 'somebodyelse');
try {
$this->assertFalse($cache->acquire_lock('apple'));
// Neither store is locked by us, local store still locked.
$this->assertFalse((bool)$localstore->check_lock_state($hashedkey, $localidentifier));
$this->assertFalse((bool)$sharedstore->check_lock_state($hashedkey, $sharedidentifier));
$this->assertTrue((bool)$localstore->check_lock_state($hashedkey, 'somebodyelse'));
} finally {
$localstore->release_lock($hashedkey, 'somebodyelse');
}
// 3. Just for completion, test what happens if we do lock it.
$this->assertTrue($cache->acquire_lock('apple'));
try {
$this->assertTrue((bool)$localstore->check_lock_state($hashedkey, $localidentifier));
$this->assertTrue((bool)$sharedstore->check_lock_state($hashedkey, $sharedidentifier));
} finally {
$cache->release_lock('apple');
}
}
/**
* Test the static cache_helper method purge_stores_used_by_definition.
*/