From 3a3a8bb96d0cb7be2529ab095f305fd3b042783c Mon Sep 17 00:00:00 2001
From: Nils Adermann <naderman@naderman.de>
Date: Fri, 7 Jan 2011 20:58:28 +0100
Subject: [PATCH] [feature/system-cron] Abstract the database locking mechanism
out of cron.
Added a number of tests for the locking mechanism which can now lock
arbitrary config variables.
PHPBB3-9596
---
phpBB/cron.php | 10 +--
phpBB/includes/cron/lock.php | 104 ----------------------
phpBB/includes/lock/db.php | 158 +++++++++++++++++++++++++++++++++
tests/lock/db_test.php | 67 ++++++++++++++
tests/lock/fixtures/config.xml | 13 +++
5 files changed, 243 insertions(+), 109 deletions(-)
delete mode 100644 phpBB/includes/cron/lock.php
create mode 100644 phpBB/includes/lock/db.php
create mode 100644 tests/lock/db_test.php
create mode 100644 tests/lock/fixtures/config.xml
diff --git a/phpBB/cron.php b/phpBB/cron.php
index d1b96b12e1..80e744d358 100644
--- a/phpBB/cron.php
+++ b/phpBB/cron.php
@@ -33,9 +33,9 @@ function output_image()
// flush();
}
-function do_cron($run_tasks)
+function do_cron($cron_lock, $run_tasks)
{
- global $cron_lock, $config;
+ global $config;
foreach ($run_tasks as $task)
{
@@ -73,7 +73,7 @@ else
output_image();
}
-$cron_lock = new phpbb_cron_lock();
+$cron_lock = new phpbb_lock_db('cron_lock', $config, $db);
if ($cron_lock->lock())
{
if ($config['use_system_cron'])
@@ -103,11 +103,11 @@ if ($cron_lock->lock())
}
if ($use_shutdown_function)
{
- register_shutdown_function('do_cron', $run_tasks);
+ register_shutdown_function('do_cron', $cron_lock, $run_tasks);
}
else
{
- do_cron($run_tasks);
+ do_cron($cron_lock, $run_tasks);
}
}
else
diff --git a/phpBB/includes/cron/lock.php b/phpBB/includes/cron/lock.php
deleted file mode 100644
index 27b8b425c1..0000000000
--- a/phpBB/includes/cron/lock.php
+++ /dev/null
@@ -1,104 +0,0 @@
-<?php
-/**
-*
-* @package phpBB3
-* @copyright (c) 2010 phpBB Group
-* @license http://opensource.org/licenses/gpl-license.php GNU Public License
-*
-*/
-
-/**
-* @ignore
-*/
-if (!defined('IN_PHPBB'))
-{
- exit;
-}
-
-/**
-* Cron lock class
-* @package phpBB3
-*/
-class phpbb_cron_lock
-{
- /**
- * Unique identifier for this lock.
- *
- * @var string
- */
- private $cron_id;
-
- /**
- * Tries to acquire the cron lock by updating
- * the 'cron_lock' configuration variable in the database.
- *
- * As a lock may only be held by one process at a time, lock
- * acquisition may fail if another process is holding the lock
- * or if another process obtained the lock but never released it.
- * Locks are forcibly released after a timeout of 1 hour.
- *
- * @return bool true if lock was acquired
- * false otherwise
- */
- public function lock()
- {
- global $config, $db;
-
- if (!isset($config['cron_lock']))
- {
- set_config('cron_lock', '0', true);
- }
-
- // make sure cron doesn't run multiple times in parallel
- if ($config['cron_lock'])
- {
- // if the other process is running more than an hour already we have to assume it
- // aborted without cleaning the lock
- $time = explode(' ', $config['cron_lock']);
- $time = $time[0];
-
- if ($time + 3600 >= time())
- {
- return false;
- }
- }
-
- $this->cron_id = time() . ' ' . unique_id();
-
- $sql = 'UPDATE ' . CONFIG_TABLE . "
- SET config_value = '" . $db->sql_escape($this->cron_id) . "'
- WHERE config_name = 'cron_lock'
- AND config_value = '" . $db->sql_escape($config['cron_lock']) . "'";
- $db->sql_query($sql);
-
- // another cron process altered the table between script start and UPDATE query so exit
- if ($db->sql_affectedrows() != 1)
- {
- return false;
- }
-
- return true;
- }
-
- /**
- * Releases the cron lock.
- *
- * The lock must have been previously obtained, that is, lock() call
- * was issued and returned true.
- *
- * Note: Attempting to release a cron lock that is already released,
- * that is, calling unlock() multiple times, is harmless.
- *
- * @return void
- */
- public function unlock()
- {
- global $db;
-
- $sql = 'UPDATE ' . CONFIG_TABLE . "
- SET config_value = '0'
- WHERE config_name = 'cron_lock'
- AND config_value = '" . $db->sql_escape($this->cron_id) . "'";
- $db->sql_query($sql);
- }
-}
diff --git a/phpBB/includes/lock/db.php b/phpBB/includes/lock/db.php
new file mode 100644
index 0000000000..e9885285d9
--- /dev/null
+++ b/phpBB/includes/lock/db.php
@@ -0,0 +1,158 @@
+<?php
+/**
+*
+* @package phpBB3
+* @copyright (c) 2010 phpBB Group
+* @license http://opensource.org/licenses/gpl-license.php GNU Public License
+*
+*/
+
+/**
+* @ignore
+*/
+if (!defined('IN_PHPBB'))
+{
+ exit;
+}
+
+/**
+* Database locking class
+* @package phpBB3
+*/
+class phpbb_lock_db
+{
+ /**
+ * Name of the config variable this lock uses
+ * @var string
+ */
+ private $config_name;
+
+ /**
+ * Unique identifier for this lock.
+ *
+ * @var string
+ */
+ private $unique_id;
+
+ /**
+ * Stores the state of this lock
+ * @var bool
+ */
+ private $locked;
+
+ /**
+ * The phpBB configuration
+ * @var array
+ */
+ private $config;
+
+ /**
+ * A database connection
+ * @var dbal
+ */
+ private $db;
+
+ /**
+ * Creates a named released instance of the lock.
+ *
+ * You have to call lock to actually create the lock.
+ *
+ * @param string $config_name A config variable to be used for locking
+ * @param array $config The phpBB configuration
+ * @param dbal $db A database connection
+ */
+ public function __construct($config_name, $config, dbal $db)
+ {
+ $this->config_name = $config_name;
+ $this->config = $config;
+ $this->db = $db;
+ }
+
+ /**
+ * Tries to acquire the cron lock by updating
+ * the 'cron_lock' configuration variable in the database.
+ *
+ * As a lock may only be held by one process at a time, lock
+ * acquisition may fail if another process is holding the lock
+ * or if another process obtained the lock but never released it.
+ * Locks are forcibly released after a timeout of 1 hour.
+ *
+ * @return bool true if lock was acquired
+ * false otherwise
+ */
+ public function lock()
+ {
+ if ($this->locked)
+ {
+ return true;
+ }
+
+ if (!isset($this->config[$this->config_name]))
+ {
+ set_config($this->config_name, '0', true);
+ global $config;
+ $this->config = $config;
+ }
+ $lock_value = $this->config[$this->config_name];
+
+ // make sure cron doesn't run multiple times in parallel
+ if ($lock_value)
+ {
+ // if the other process is running more than an hour already we have to assume it
+ // aborted without cleaning the lock
+ $time = explode(' ', $lock_value);
+ $time = $time[0];
+
+ if ($time + 3600 >= time())
+ {
+ return false;
+ }
+ }
+
+ $this->unique_id = time() . ' ' . unique_id();
+
+ $sql = 'UPDATE ' . CONFIG_TABLE . "
+ SET config_value = '" . $this->db->sql_escape($this->unique_id) . "'
+ WHERE config_name = '" . $this->db->sql_escape($this->config_name) . "'
+ 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;
+ }
+
+ /**
+ * Releases the cron lock.
+ *
+ * The lock must have been previously obtained, that is, lock() call
+ * was issued and returned true.
+ *
+ * Note: Attempting to release a cron lock that is already released,
+ * that is, calling unlock() multiple times, is harmless.
+ *
+ * @return void
+ */
+ public function unlock()
+ {
+ if ($this->locked)
+ {
+ $sql = 'UPDATE ' . CONFIG_TABLE . "
+ 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;
+ }
+ }
+}
diff --git a/tests/lock/db_test.php b/tests/lock/db_test.php
new file mode 100644
index 0000000000..0702a2c21e
--- /dev/null
+++ b/tests/lock/db_test.php
@@ -0,0 +1,67 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2010 phpBB Group
+* @license http://opensource.org/licenses/gpl-license.php GNU Public License
+*
+*/
+
+require_once __DIR__ . '/../../phpBB/includes/functions.php';
+
+class phpbb_lock_db_test extends phpbb_database_test_case
+{
+ private $db;
+ private $config;
+ private $lock;
+
+ public function getDataSet()
+ {
+ return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/config.xml');
+ }
+
+ public function setUp()
+ {
+ global $db, $config;
+
+ $db = $this->db = $this->new_dbal();
+ $config = $this->config = array('rand_seed' => '', 'rand_seed_last_update' => '0');
+ $this->lock = new phpbb_lock_db('test_lock', $this->config, $this->db);
+ }
+
+ public function test_new_lock()
+ {
+ global $config;
+
+ $this->assertTrue($this->lock->lock());
+ $this->assertTrue(isset($config['test_lock']), 'Lock was created');
+
+ $lock2 = new phpbb_lock_db('test_lock', $config, $this->db);
+ $this->assertFalse($lock2->lock());
+
+ $this->lock->unlock();
+ $this->assertEquals('0', $config['test_lock'], 'Lock was released');
+ }
+
+ public function test_expire_lock()
+ {
+ $lock = new phpbb_lock_db('foo_lock', $this->config, $this->db);
+ $this->assertTrue($lock->lock());
+ }
+
+ public function test_double_lock()
+ {
+ global $config;
+
+ $this->assertTrue($this->lock->lock());
+ $this->assertTrue(isset($config['test_lock']), 'Lock was created');
+
+ $value = $config['test_lock'];
+
+ $this->assertTrue($this->lock->lock());
+ $this->assertEquals($value, $config['test_lock'], 'Second lock was ignored');
+
+ $this->lock->unlock();
+ $this->assertEquals('0', $config['test_lock'], 'Lock was released');
+ }
+}
diff --git a/tests/lock/fixtures/config.xml b/tests/lock/fixtures/config.xml
new file mode 100644
index 0000000000..f36c8b929a
--- /dev/null
+++ b/tests/lock/fixtures/config.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<dataset>
+ <table name="phpbb_config">
+ <column>config_name</column>
+ <column>config_value</column>
+ <column>is_dynamic</column>
+ <row>
+ <value>foo_lock</value>
+ <value>1 abcd</value>
+ <value>1</value>
+ </row>
+ </table>
+</dataset>