[feature/system-cron] Make use of the new config class in locks.

PHPBB3-9596
This commit is contained in:
Nils Adermann 2011-01-12 22:43:09 +01:00 committed by Oleg Pudeyev
parent 311a7ff753
commit 165b0ec0b2
2 changed files with 42 additions and 46 deletions

View file

@ -42,7 +42,7 @@ class phpbb_lock_db
/** /**
* The phpBB configuration * The phpBB configuration
* @var array * @var phpbb_config
*/ */
private $config; private $config;
@ -61,7 +61,7 @@ class phpbb_lock_db
* @param array $config The phpBB configuration * @param array $config The phpBB configuration
* @param dbal $db A database connection * @param dbal $db A database connection
*/ */
public function __construct($config_name, $config, dbal $db) public function __construct($config_name, phpbb_config $config, dbal $db)
{ {
$this->config_name = $config_name; $this->config_name = $config_name;
$this->config = $config; $this->config = $config;
@ -69,8 +69,8 @@ class phpbb_lock_db
} }
/** /**
* Tries to acquire the cron lock by updating * Tries to acquire the lock by updating
* the 'cron_lock' configuration variable in the database. * the configuration variable in the database.
* *
* As a lock may only be held by one process at a time, lock * As a lock may only be held by one process at a time, lock
* acquisition may fail if another process is holding the lock * acquisition may fail if another process is holding the lock
@ -89,13 +89,11 @@ class phpbb_lock_db
if (!isset($this->config[$this->config_name])) if (!isset($this->config[$this->config_name]))
{ {
set_config($this->config_name, '0', true); $this->config->set($this->config_name, '0', false);
global $config;
$this->config = $config;
} }
$lock_value = $this->config[$this->config_name]; $lock_value = $this->config[$this->config_name];
// make sure cron doesn't run multiple times in parallel // make sure lock cannot be acquired by multiple processes
if ($lock_value) if ($lock_value)
{ {
// if the other process is running more than an hour already we have to assume it // if the other process is running more than an hour already we have to assume it
@ -111,33 +109,20 @@ class phpbb_lock_db
$this->unique_id = time() . ' ' . unique_id(); $this->unique_id = time() . ' ' . unique_id();
$sql = 'UPDATE ' . CONFIG_TABLE . " // try to update the config value, if it was already modified by another
SET config_value = '" . $this->db->sql_escape($this->unique_id) . "' // process we failed to acquire the lock.
WHERE config_name = '" . $this->db->sql_escape($this->config_name) . "' $this->locked = $this->config->set_atomic($this->config_name, $lock_value, $this->unique_id, false);
AND config_value = '" . $this->db->sql_escape($lock_value) . "'";
$this->db->sql_query($sql);
if ($this->db->sql_affectedrows())
{
$this->locked = true;
}
else
{
// another cron process altered the table between script start and
// UPDATE query so return false
$this->locked = false;
}
return $this->locked; return $this->locked;
} }
/** /**
* Releases the cron lock. * Releases the lock.
* *
* The lock must have been previously obtained, that is, lock() call * The lock must have been previously obtained, that is, lock() call
* was issued and returned true. * was issued and returned true.
* *
* Note: Attempting to release a cron lock that is already released, * Note: Attempting to release a lock that is already released,
* that is, calling unlock() multiple times, is harmless. * that is, calling unlock() multiple times, is harmless.
* *
* @return void * @return void
@ -146,12 +131,7 @@ class phpbb_lock_db
{ {
if ($this->locked) if ($this->locked)
{ {
$sql = 'UPDATE ' . CONFIG_TABLE . " $this->config->set_atomic($this->config_name, $this->unique_id, '0', false);
SET config_value = '0'
WHERE config_name = '" . $this->db->sql_escape($this->config_name) . "'
AND config_value = '" . $this->db->sql_escape($this->unique_id) . "'";
$this->db->sql_query($sql);
$this->locked = false; $this->locked = false;
} }
} }

View file

@ -25,22 +25,21 @@ class phpbb_lock_db_test extends phpbb_database_test_case
global $db, $config; global $db, $config;
$db = $this->db = $this->new_dbal(); $db = $this->db = $this->new_dbal();
$config = $this->config = array('rand_seed' => '', 'rand_seed_last_update' => '0'); $config = $this->config = new phpbb_config(array('rand_seed' => '', 'rand_seed_last_update' => '0'));
set_config(null, null, null, $this->config);
$this->lock = new phpbb_lock_db('test_lock', $this->config, $this->db); $this->lock = new phpbb_lock_db('test_lock', $this->config, $this->db);
} }
public function test_new_lock() public function test_new_lock()
{ {
global $config;
$this->assertTrue($this->lock->lock()); $this->assertTrue($this->lock->lock());
$this->assertTrue(isset($config['test_lock']), 'Lock was created'); $this->assertTrue(isset($this->config['test_lock']), 'Lock was created');
$lock2 = new phpbb_lock_db('test_lock', $config, $this->db); $lock2 = new phpbb_lock_db('test_lock', $this->config, $this->db);
$this->assertFalse($lock2->lock()); $this->assertFalse($lock2->lock());
$this->lock->unlock(); $this->lock->unlock();
$this->assertEquals('0', $config['test_lock'], 'Lock was released'); $this->assertEquals('0', $this->config['test_lock'], 'Lock was released');
} }
public function test_expire_lock() public function test_expire_lock()
@ -51,17 +50,34 @@ class phpbb_lock_db_test extends phpbb_database_test_case
public function test_double_lock() public function test_double_lock()
{ {
global $config; $this->assertTrue($this->lock->lock());
$this->assertTrue(isset($this->config['test_lock']), 'Lock was created');
$value = $this->config['test_lock'];
$this->assertTrue($this->lock->lock()); $this->assertTrue($this->lock->lock());
$this->assertTrue(isset($config['test_lock']), 'Lock was created'); $this->assertEquals($value, $this->config['test_lock'], 'Second lock was ignored');
$value = $config['test_lock'];
$this->assertTrue($this->lock->lock());
$this->assertEquals($value, $config['test_lock'], 'Second lock was ignored');
$this->lock->unlock(); $this->lock->unlock();
$this->assertEquals('0', $config['test_lock'], 'Lock was released'); $this->assertEquals('0', $this->config['test_lock'], 'Lock was released');
}
public function test_double_unlock()
{
$this->assertTrue($this->lock->lock());
$this->assertFalse(empty($this->config['test_lock']), 'First lock is acquired');
$this->lock->unlock();
$this->assertEquals('0', $this->config['test_lock'], 'First lock is released');
$lock2 = new phpbb_lock_db('test_lock', $this->config, $this->db);
$this->assertTrue($lock2->lock());
$this->assertFalse(empty($this->config['test_lock']), 'Second lock is acquired');
$this->lock->unlock();
$this->assertFalse(empty($this->config['test_lock']), 'Double release of first lock is ignored');
$lock2->unlock();
$this->assertEquals('0', $this->config['test_lock'], 'Second lock is released');
} }
} }