Skip to content

Commit bb56765

Browse files
memleakdmemleakd
committed
refactor(lock): move lock stores out of cache handlers
- introduce cache lock store provider and dedicated File, Redis, and Predis lock stores - keep cache handlers responsible only for exposing their lock store - update LockManager to resolve lock support through cache lock store providers - refresh memoized Redis and Predis lock stores after reconnect - update Redis and Predis lock tests for provider-based stores and reconnect behavior - document custom lock store extension expectations Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
1 parent 6667132 commit bb56765

13 files changed

Lines changed: 489 additions & 320 deletions

File tree

‎system/Cache/Handlers/FileHandler.php‎

Lines changed: 10 additions & 161 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
use CodeIgniter\Cache\Exceptions\CacheException;
1717
use CodeIgniter\Cache\LockStoreInterface;
18+
use CodeIgniter\Cache\LockStoreProvider;
19+
use CodeIgniter\Cache\LockStores\FileLockStore;
1820
use CodeIgniter\I18n\Time;
1921
use Config\Cache;
2022
use Throwable;
@@ -24,7 +26,7 @@
2426
*
2527
* @see \CodeIgniter\Cache\Handlers\FileHandlerTest
2628
*/
27-
class FileHandler extends BaseHandler implements LockStoreInterface
29+
class FileHandler extends BaseHandler implements LockStoreProvider
2830
{
2931
/**
3032
* Maximum key length.
@@ -48,6 +50,8 @@ class FileHandler extends BaseHandler implements LockStoreInterface
4850
*/
4951
protected $mode;
5052

53+
private ?LockStoreInterface $lockStore = null;
54+
5155
/**
5256
* Note: Use `CacheFactory::getHandler()` to instantiate.
5357
*
@@ -156,78 +160,6 @@ public function decrement(string $key, int $offset = 1): bool|int
156160
return $this->increment($key, -$offset);
157161
}
158162

159-
public function acquireLock(string $key, string $owner, int $ttl): bool
160-
{
161-
return $this->withLockFile($key, static function ($handle) use ($owner, $ttl): bool {
162-
$data = self::readLockData($handle);
163-
$now = Time::now()->getTimestamp();
164-
165-
if ($data !== null && $data['expires'] > $now) {
166-
return false;
167-
}
168-
169-
return self::writeLockData($handle, $owner, $now + $ttl);
170-
});
171-
}
172-
173-
public function releaseLock(string $key, string $owner): bool
174-
{
175-
return $this->withLockFile($key, static function ($handle) use ($owner): bool {
176-
$data = self::readLockData($handle);
177-
178-
if ($data === null || $data['owner'] !== $owner) {
179-
return false;
180-
}
181-
182-
return self::clearLockFile($handle);
183-
});
184-
}
185-
186-
public function forceReleaseLock(string $key): bool
187-
{
188-
return ! is_file($this->path . static::validateKey($key, $this->prefix))
189-
|| $this->withLockFile($key, static fn ($handle): bool => self::clearLockFile($handle), false);
190-
}
191-
192-
public function refreshLock(string $key, string $owner, int $ttl): bool
193-
{
194-
return $this->withLockFile($key, static function ($handle) use ($owner, $ttl): bool {
195-
$data = self::readLockData($handle);
196-
$now = Time::now()->getTimestamp();
197-
198-
if ($data === null || $data['owner'] !== $owner || $data['expires'] <= $now) {
199-
return false;
200-
}
201-
202-
return self::writeLockData($handle, $owner, $now + $ttl);
203-
});
204-
}
205-
206-
public function getLockOwner(string $key): ?string
207-
{
208-
$owner = null;
209-
210-
$this->withLockFile($key, static function ($handle) use (&$owner): bool {
211-
$data = self::readLockData($handle);
212-
213-
if ($data === null) {
214-
return true;
215-
}
216-
217-
if ($data['expires'] <= Time::now()->getTimestamp()) {
218-
self::clearLockFile($handle);
219-
220-
return true;
221-
}
222-
223-
$owner = $data['owner'];
224-
225-
return true;
226-
}, false);
227-
228-
return $owner;
229-
}
230-
231163
public function clean(): bool
232164
{
233165
return delete_files($this->path, false, true);
@@ -258,6 +190,11 @@ public function isSupported(): bool
258190
return is_writable($this->path);
259191
}
260192

193+
public function lockStore(): LockStoreInterface
194+
{
195+
return $this->lockStore ??= new FileLockStore($this->path, $this->mode, $this->prefix);
196+
}
197+
261198
/**
262199
* Does the heavy lifting of actually retrieving the file and
263200
* verifying its age.
@@ -302,92 +239,4 @@ protected function getItem(string $filename): array|false
302239

303240
return $data;
304241
}
305-
306-
/**
307-
* @param callable(resource): bool $callback
308-
*/
309-
private function withLockFile(string $key, callable $callback, bool $create = true): bool
310-
{
311-
$key = static::validateKey($key, $this->prefix);
312-
$handle = @fopen($this->path . $key, $create ? 'c+b' : 'r+b');
313-
314-
if ($handle === false) {
315-
return false;
316-
}
317-
318-
try {
319-
if (! flock($handle, LOCK_EX)) {
320-
return false;
321-
}
322-
323-
return $callback($handle);
324-
} finally {
325-
flock($handle, LOCK_UN);
326-
fclose($handle);
327-
328-
if (is_file($this->path . $key)) {
329-
try {
330-
chmod($this->path . $key, $this->mode);
331-
} catch (Throwable $e) {
332-
log_message('debug', 'Failed to set mode on cache lock file: ' . $e);
333-
}
334-
}
335-
}
336-
}
337-
338-
/**
339-
* @param resource $handle
340-
*
341-
* @return array{owner: string, expires: int}|null
342-
*/
343-
private static function readLockData($handle): ?array
344-
{
345-
rewind($handle);
346-
347-
$content = stream_get_contents($handle);
348-
349-
if ($content === false || $content === '') {
350-
return null;
351-
}
352-
353-
try {
354-
$data = unserialize($content);
355-
} catch (Throwable) {
356-
return null;
357-
}
358-
359-
if (! is_array($data) || ! isset($data['owner'], $data['expires']) || ! is_string($data['owner']) || ! is_int($data['expires'])) {
360-
return null;
361-
}
362-
363-
return $data;
364-
}
365-
366-
/**
367-
* @param resource $handle
368-
*/
369-
private static function writeLockData($handle, string $owner, int $expires): bool
370-
{
371-
rewind($handle);
372-
373-
if (! ftruncate($handle, 0)) {
374-
return false;
375-
}
376-
377-
if (fwrite($handle, serialize(['owner' => $owner, 'expires' => $expires])) === false) {
378-
return false;
379-
}
380-
381-
return fflush($handle);
382-
}
383-
384-
/**
385-
* @param resource $handle
386-
*/
387-
private static function clearLockFile($handle): bool
388-
{
389-
rewind($handle);
390-
391-
return ftruncate($handle, 0) && fflush($handle);
392-
}
393242
}

‎system/Cache/Handlers/PredisHandler.php‎

Lines changed: 12 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
namespace CodeIgniter\Cache\Handlers;
1515

1616
use CodeIgniter\Cache\LockStoreInterface;
17+
use CodeIgniter\Cache\LockStoreProvider;
18+
use CodeIgniter\Cache\LockStores\PredisLockStore;
1719
use CodeIgniter\Exceptions\CriticalError;
1820
use CodeIgniter\I18n\Time;
1921
use Config\Cache;
@@ -27,7 +29,7 @@
2729
*
2830
* @see \CodeIgniter\Cache\Handlers\PredisHandlerTest
2931
*/
30-
class PredisHandler extends BaseHandler implements LockStoreInterface
32+
class PredisHandler extends BaseHandler implements LockStoreProvider
3133
{
3234
/**
3335
* Default config
@@ -59,6 +61,8 @@ class PredisHandler extends BaseHandler implements LockStoreInterface
5961
*/
6062
protected $redis;
6163

64+
private ?LockStoreInterface $lockStore = null;
65+
6266
/**
6367
* Note: Use `CacheFactory::getHandler()` to instantiate.
6468
*/
@@ -72,7 +76,8 @@ public function __construct(Cache $config)
7276
public function initialize(): void
7377
{
7478
try {
75-
$this->redis = new Client($this->config, ['prefix' => $this->prefix]);
79+
$this->redis = new Client($this->config, ['prefix' => $this->prefix]);
80+
$this->lockStore = null;
7681
$this->redis->time();
7782
} catch (Exception $e) {
7883
throw new CriticalError('Cache: Predis connection refused (' . $e->getMessage() . ').', $e->getCode(), $e);
@@ -168,60 +173,6 @@ public function decrement(string $key, int $offset = 1): int
168173
return $this->redis->hincrby($key, 'data', -$offset);
169174
}
170175

171-
public function acquireLock(string $key, string $owner, int $ttl): bool
172-
{
173-
$key = static::validateKey($key);
174-
$result = $this->redis->set($key, $owner, 'EX', $ttl, 'NX');
175-
176-
return $result instanceof Status && $result->getPayload() === 'OK';
177-
}
178-
179-
public function releaseLock(string $key, string $owner): bool
180-
{
181-
$key = static::validateKey($key);
182-
183-
$script = <<<'LUA'
184-
if redis.call("get", KEYS[1]) == ARGV[1] then
185-
return redis.call("del", KEYS[1])
186-
end
187-
188-
return 0
189-
LUA;
190-
191-
return $this->redis->eval($script, 1, $key, $owner) === 1;
192-
}
193-
194-
public function forceReleaseLock(string $key): bool
195-
{
196-
$key = static::validateKey($key);
197-
$deleted = $this->redis->del($key);
198-
199-
return is_int($deleted) && $deleted >= 0;
200-
}
201-
202-
public function refreshLock(string $key, string $owner, int $ttl): bool
203-
{
204-
$key = static::validateKey($key);
205-
206-
$script = <<<'LUA'
207-
if redis.call("get", KEYS[1]) == ARGV[1] then
208-
return redis.call("expire", KEYS[1], ARGV[2])
209-
end
210-
211-
return 0
212-
LUA;
213-
214-
return $this->redis->eval($script, 1, $key, $owner, (string) $ttl) === 1;
215-
}
216-
217-
public function getLockOwner(string $key): ?string
218-
{
219-
$key = static::validateKey($key);
220-
$owner = $this->redis->get($key);
221-
222-
return is_string($owner) ? $owner : null;
223-
}
224-
225176
public function clean(): bool
226177
{
227178
return $this->redis->flushdb()->getPayload() === 'OK';
@@ -257,6 +208,11 @@ public function isSupported(): bool
257208
return class_exists(Client::class);
258209
}
259210

211+
public function lockStore(): LockStoreInterface
212+
{
213+
return $this->lockStore ??= new PredisLockStore($this->redis);
214+
}
215+
260216
public function ping(): bool
261217
{
262218
try {

0 commit comments

Comments
 (0)