From 00b05099f33257bd33ebb1548bec3664b653e235 Mon Sep 17 00:00:00 2001 From: Zankaria Date: Fri, 16 Feb 2024 14:18:55 +0100 Subject: [PATCH 1/6] Format lock.php --- inc/lock.php | 68 +++++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/inc/lock.php b/inc/lock.php index 4fb2f5df..23c0bce4 100644 --- a/inc/lock.php +++ b/inc/lock.php @@ -1,39 +1,43 @@ f = fopen("tmp/locks/$key", "w"); - } - } + $this->f = fopen("tmp/locks/$key", "w"); + } + } - // Get a shared lock - function get($nonblock = false) { global $config; - if ($config['lock']['enabled'] == 'fs') { - $wouldblock = false; - flock($this->f, LOCK_SH | ($nonblock ? LOCK_NB : 0), $wouldblock); - if ($nonblock && $wouldblock) return false; - } - return $this; - } + // Get a shared lock + function get($nonblock = false) { + global $config; + if ($config['lock']['enabled'] == 'fs') { + $wouldblock = false; + flock($this->f, LOCK_SH | ($nonblock ? LOCK_NB : 0), $wouldblock); + if ($nonblock && $wouldblock) return false; + } + return $this; + } - // Get an exclusive lock - function get_ex($nonblock = false) { global $config; - if ($config['lock']['enabled'] == 'fs') { - $wouldblock = false; - flock($this->f, LOCK_EX | ($nonblock ? LOCK_NB : 0), $wouldblock); - if ($nonblock && $wouldblock) return false; - } - return $this; - } + // Get an exclusive lock + function get_ex($nonblock = false) { + global $config; + if ($config['lock']['enabled'] == 'fs') { + $wouldblock = false; + flock($this->f, LOCK_EX | ($nonblock ? LOCK_NB : 0), $wouldblock); + if ($nonblock && $wouldblock) return false; + } + return $this; + } - // Free a lock - function free() { global $config; - if ($config['lock']['enabled'] == 'fs') { - flock($this->f, LOCK_UN); - } - return $this; - } + // Free a lock + function free() { + global $config; + if ($config['lock']['enabled'] == 'fs') { + flock($this->f, LOCK_UN); + } + return $this; + } } From 760431606d1309180929eda8f203d65a42039bf6 Mon Sep 17 00:00:00 2001 From: Zankaria Date: Fri, 16 Feb 2024 14:47:20 +0100 Subject: [PATCH 2/6] Refactor lock.php --- inc/lock.php | 105 +++++++++++++++++++++++++++++++++++--------------- inc/queue.php | 2 +- 2 files changed, 74 insertions(+), 33 deletions(-) diff --git a/inc/lock.php b/inc/lock.php index 23c0bce4..5a83562a 100644 --- a/inc/lock.php +++ b/inc/lock.php @@ -1,43 +1,84 @@ f = fopen("tmp/locks/$key", "w"); +class Locks { + private static function filesystem(string $key): Lock|false { + $key = str_replace('/', '::', $key); + $key = str_replace("\0", '', $key); + + $fd = fopen("tmp/locks/$key", "w"); + if ($fd === false) { + return false; } + + return new class($fd) implements Lock { + // Resources have no type in php. + private mixed $f; + + + function __construct($fd) { + $this->f = $fd; + } + + public function get(bool $nonblock = false): Lock|false { + $wouldblock = false; + flock($this->f, LOCK_SH | ($nonblock ? LOCK_NB : 0), $wouldblock); + if ($nonblock && $wouldblock) { + return false; + } + return $this; + } + + public function get_ex(bool $nonblock = false): Lock|false { + $wouldblock = false; + flock($this->f, LOCK_EX | ($nonblock ? LOCK_NB : 0), $wouldblock); + if ($nonblock && $wouldblock) { + return false; + } + return $this; + } + + public function free(): Lock { + flock($this->f, LOCK_UN); + return $this; + } + }; } - // Get a shared lock - function get($nonblock = false) { - global $config; - if ($config['lock']['enabled'] == 'fs') { - $wouldblock = false; - flock($this->f, LOCK_SH | ($nonblock ? LOCK_NB : 0), $wouldblock); - if ($nonblock && $wouldblock) return false; - } - return $this; + /** + * No-op. Can be used for mocking. + */ + public static function none(): Lock|false { + return new class() implements Lock { + public function get(bool $nonblock = false): Lock|false { + return $this; + } + + public function get_ex(bool $nonblock = false): Lock|false { + return $this; + } + + public function free(): Lock { + return $this; + } + }; } - // Get an exclusive lock - function get_ex($nonblock = false) { - global $config; + public static function get_lock(array $config, string $key): Lock|false { if ($config['lock']['enabled'] == 'fs') { - $wouldblock = false; - flock($this->f, LOCK_EX | ($nonblock ? LOCK_NB : 0), $wouldblock); - if ($nonblock && $wouldblock) return false; + return self::filesystem($key); + } else { + return self::none(); } - return $this; - } - - // Free a lock - function free() { - global $config; - if ($config['lock']['enabled'] == 'fs') { - flock($this->f, LOCK_UN); - } - return $this; } } + +interface Lock { + // Get a shared lock + public function get(bool $nonblock = false): Lock|false; + + // Get an exclusive lock + public function get_ex(bool $nonblock = false): Lock|false; + + // Free a lock + public function free(): Lock; +} diff --git a/inc/queue.php b/inc/queue.php index 66305b3b..a3873491 100644 --- a/inc/queue.php +++ b/inc/queue.php @@ -3,7 +3,7 @@ class Queue { function __construct($key) { global $config; if ($config['queue']['enabled'] == 'fs') { - $this->lock = new Lock($key); + $this->lock = Locks::get_lock($config, $key); $key = str_replace('/', '::', $key); $key = str_replace("\0", '', $key); $this->key = "tmp/queue/$key/"; From 55034762b07cdcb143c4daa42ba26c52260094a9 Mon Sep 17 00:00:00 2001 From: Zankaria Date: Fri, 16 Feb 2024 14:48:42 +0100 Subject: [PATCH 3/6] Format queue.php --- inc/queue.php | 74 +++++++++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/inc/queue.php b/inc/queue.php index a3873491..2801170e 100644 --- a/inc/queue.php +++ b/inc/queue.php @@ -1,49 +1,49 @@ lock = Locks::get_lock($config, $key); - $key = str_replace('/', '::', $key); - $key = str_replace("\0", '', $key); - $this->key = "tmp/queue/$key/"; - } - } + function __construct($key) { global $config; + if ($config['queue']['enabled'] == 'fs') { + $this->lock = Locks::get_lock($config, $key); + $key = str_replace('/', '::', $key); + $key = str_replace("\0", '', $key); + $this->key = "tmp/queue/$key/"; + } + } - function push($str) { global $config; - if ($config['queue']['enabled'] == 'fs') { - $this->lock->get_ex(); - file_put_contents($this->key.microtime(true), $str); - $this->lock->free(); - } - return $this; - } + function push($str) { global $config; + if ($config['queue']['enabled'] == 'fs') { + $this->lock->get_ex(); + file_put_contents($this->key.microtime(true), $str); + $this->lock->free(); + } + return $this; + } - function pop($n = 1) { global $config; - if ($config['queue']['enabled'] == 'fs') { - $this->lock->get_ex(); - $dir = opendir($this->key); - $paths = array(); - while ($n > 0) { - $path = readdir($dir); - if ($path === FALSE) break; - elseif ($path == '.' || $path == '..') continue; - else { $paths[] = $path; $n--; } - } - $out = array(); - foreach ($paths as $v) { - $out []= file_get_contents($this->key.$v); - unlink($this->key.$v); - } - $this->lock->free(); - return $out; - } - } + function pop($n = 1) { global $config; + if ($config['queue']['enabled'] == 'fs') { + $this->lock->get_ex(); + $dir = opendir($this->key); + $paths = array(); + while ($n > 0) { + $path = readdir($dir); + if ($path === FALSE) break; + elseif ($path == '.' || $path == '..') continue; + else { $paths[] = $path; $n--; } + } + $out = array(); + foreach ($paths as $v) { + $out []= file_get_contents($this->key.$v); + unlink($this->key.$v); + } + $this->lock->free(); + return $out; + } + } } // Don't use the constructor. Use the get_queue function. $queues = array(); function get_queue($name) { global $queues; - return $queues[$name] = isset ($queues[$name]) ? $queues[$name] : new Queue($name); + return $queues[$name] = isset ($queues[$name]) ? $queues[$name] : new Queue($name); } From e61ed35aa06c633db93903d64244a3f82221586f Mon Sep 17 00:00:00 2001 From: Zankaria Date: Fri, 16 Feb 2024 15:18:17 +0100 Subject: [PATCH 4/6] Refactor queue.php --- inc/functions.php | 12 ++++- inc/queue.php | 121 ++++++++++++++++++++++++++++++++-------------- 2 files changed, 95 insertions(+), 38 deletions(-) diff --git a/inc/functions.php b/inc/functions.php index 50e0ca38..5890da8a 100755 --- a/inc/functions.php +++ b/inc/functions.php @@ -2918,8 +2918,16 @@ function generation_strategy($fun, $array=array()) { global $config; return 'rebuild'; case 'defer': // Ok, it gets interesting here :) - get_queue('generate')->push(serialize(array('build', $fun, $array, $action))); - return 'ignore'; + $queue = Queues::get_queue($config, 'generate'); + if ($queue === false) { + if ($config['syslog']) { + _syslog(LOG_ERR, "Could not initialize generate queue, falling back to immediate rebuild strategy"); + } + return 'rebuild'; + } else { + $queue->push(serialize(array('build', $fun, $array, $action))); + return 'ignore'; + } case 'build_on_load': return 'delete'; } diff --git a/inc/queue.php b/inc/queue.php index 2801170e..0ecd1e3c 100644 --- a/inc/queue.php +++ b/inc/queue.php @@ -1,49 +1,98 @@ lock = Locks::get_lock($config, $key); - $key = str_replace('/', '::', $key); - $key = str_replace("\0", '', $key); - $this->key = "tmp/queue/$key/"; - } +class Queues { + private static $queues = array(); + + + /** + * This queue implementation isn't actually ordered, so it works more as a "bag". + */ + private static function filesystem(string $key, Lock $lock): Queue { + $key = str_replace('/', '::', $key); + $key = str_replace("\0", '', $key); + $key = "tmp/queue/$key/"; + + return new class($key, $lock) implements Queue { + private Lock $lock; + private string $key; + + + function __construct(string $key, Lock $lock) { + $this->lock = $lock; + $this->key = $key; + } + + public function push(string $str): Queue { + $this->lock->get_ex(); + file_put_contents($this->key . microtime(true), $str); + $this->lock->free(); + return $this; + } + + public function pop(int $n = 1): array { + $this->lock->get_ex(); + $dir = opendir($this->key); + $paths = array(); + + while ($n > 0) { + $path = readdir($dir); + if ($path === false) { + break; + } elseif ($path == '.' || $path == '..') { + continue; + } else { + $paths[] = $path; + $n--; + } + } + + $out = array(); + foreach ($paths as $v) { + $out[] = file_get_contents($this->key . $v); + unlink($this->key . $v); + } + + $this->lock->free(); + return $out; + } + }; } - function push($str) { global $config; - if ($config['queue']['enabled'] == 'fs') { - $this->lock->get_ex(); - file_put_contents($this->key.microtime(true), $str); - $this->lock->free(); - } - return $this; + /** + * No-op. Can be used for mocking. + */ + public static function none(): Queue { + return new class() implements Queue { + public function push(string $str): Queue { + return $this; + } + + public function pop(int $n = 1): array { + return array(); + } + }; } - function pop($n = 1) { global $config; - if ($config['queue']['enabled'] == 'fs') { - $this->lock->get_ex(); - $dir = opendir($this->key); - $paths = array(); - while ($n > 0) { - $path = readdir($dir); - if ($path === FALSE) break; - elseif ($path == '.' || $path == '..') continue; - else { $paths[] = $path; $n--; } + public static function get_queue(array $config, string $name): Queue|false { + if (!isset(self::$queues[$name])) { + if ($config['queue']['enabled'] == 'fs') { + $lock = Locks::get_lock($config, $name); + if ($lock === false) { + return false; + } + self::$queues[$name] = self::filesystem($name, $lock); + } else { + self::$queues[$name] = self::none(); } - $out = array(); - foreach ($paths as $v) { - $out []= file_get_contents($this->key.$v); - unlink($this->key.$v); - } - $this->lock->free(); - return $out; } + return self::$queues[$name]; } } -// Don't use the constructor. Use the get_queue function. -$queues = array(); +interface Queue { + // Push a string in the queue. + public function push(string $str): Queue; -function get_queue($name) { global $queues; - return $queues[$name] = isset ($queues[$name]) ? $queues[$name] : new Queue($name); + // Get a string from the queue. + public function pop(int $n = 1): array; } From 09dc44ec406cdfc12d4d9a0be7f37caa1e5a7ba8 Mon Sep 17 00:00:00 2001 From: Zankaria Date: Mon, 11 Mar 2024 10:31:19 +0100 Subject: [PATCH 5/6] functions.php: trim --- inc/functions.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/inc/functions.php b/inc/functions.php index 5890da8a..ad69e55a 100755 --- a/inc/functions.php +++ b/inc/functions.php @@ -428,10 +428,10 @@ function rebuildThemes($action, $boardname = false) { $board = $_board; // Reload the locale - if ($config['locale'] != $current_locale) { - $current_locale = $config['locale']; - init_locale($config['locale']); - } + if ($config['locale'] != $current_locale) { + $current_locale = $config['locale']; + init_locale($config['locale']); + } if (PHP_SAPI === 'cli') { echo "Rebuilding theme ".$theme['theme']."... "; @@ -450,8 +450,8 @@ function rebuildThemes($action, $boardname = false) { // Reload the locale if ($config['locale'] != $current_locale) { - $current_locale = $config['locale']; - init_locale($config['locale']); + $current_locale = $config['locale']; + init_locale($config['locale']); } } From f47332cdffb00cfe33cb5371de6e0a3d300e25f7 Mon Sep 17 00:00:00 2001 From: Zankaria Date: Fri, 16 Feb 2024 15:48:18 +0100 Subject: [PATCH 6/6] Allow queue push to fail gracefully --- inc/functions.php | 11 ++++++++--- inc/queue.php | 12 ++++++------ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/inc/functions.php b/inc/functions.php index ad69e55a..71231203 100755 --- a/inc/functions.php +++ b/inc/functions.php @@ -2924,10 +2924,15 @@ function generation_strategy($fun, $array=array()) { global $config; _syslog(LOG_ERR, "Could not initialize generate queue, falling back to immediate rebuild strategy"); } return 'rebuild'; - } else { - $queue->push(serialize(array('build', $fun, $array, $action))); - return 'ignore'; } + $ret = $queue->push(serialize(array('build', $fun, $array, $action))); + if ($ret === false) { + if ($config['syslog']) { + _syslog(LOG_ERR, "Could not push item in the queue, falling back to immediate rebuild strategy"); + } + return 'rebuild'; + } + return 'ignore'; case 'build_on_load': return 'delete'; } diff --git a/inc/queue.php b/inc/queue.php index 0ecd1e3c..a5905c84 100644 --- a/inc/queue.php +++ b/inc/queue.php @@ -22,11 +22,11 @@ class Queues { $this->key = $key; } - public function push(string $str): Queue { + public function push(string $str): bool { $this->lock->get_ex(); - file_put_contents($this->key . microtime(true), $str); + $ret = file_put_contents($this->key . microtime(true), $str); $this->lock->free(); - return $this; + return $ret !== false; } public function pop(int $n = 1): array { @@ -63,8 +63,8 @@ class Queues { */ public static function none(): Queue { return new class() implements Queue { - public function push(string $str): Queue { - return $this; + public function push(string $str): bool { + return true; } public function pop(int $n = 1): array { @@ -91,7 +91,7 @@ class Queues { interface Queue { // Push a string in the queue. - public function push(string $str): Queue; + public function push(string $str): bool; // Get a string from the queue. public function pop(int $n = 1): array;