Skip to content

Commit 38ea069

Browse files
bshafferbshaffer
authored
feat: add semaphore locking to Sysv cache (#640)
1 parent b6b6966 commit 38ea069

File tree

4 files changed

+203
-28
lines changed

4 files changed

+203
-28
lines changed

‎src/Cache/SysVCacheItemPool.php‎

Lines changed: 127 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
use Psr\Cache\CacheItemInterface;
2020
use Psr\Cache\CacheItemPoolInterface;
21+
use SysvSemaphore;
22+
use SysvSharedMemory;
2123

2224
/**
2325
* SystemV shared memory based CacheItemPool implementation.
@@ -32,6 +34,8 @@ class SysVCacheItemPool implements CacheItemPoolInterface
3234

3335
const DEFAULT_PROJ = 'A';
3436

37+
const DEFAULT_SEM_PROJ = 'B';
38+
3539
const DEFAULT_MEMSIZE = 10000;
3640

3741
const DEFAULT_PERM = 0600;
@@ -61,6 +65,18 @@ class SysVCacheItemPool implements CacheItemPoolInterface
6165
*/
6266
private $hasLoadedItems = false;
6367

68+
/**
69+
* @var SysvSemaphore|false
70+
*/
71+
private SysvSemaphore|false $semId = false;
72+
73+
/**
74+
* Maintain the process which is currently holding the semaphore to prevent deadlock.
75+
*
76+
* @var int|null
77+
*/
78+
private ?int $lockOwnerPid = null;
79+
6480
/**
6581
* Create a SystemV shared memory based CacheItemPool.
6682
*
@@ -70,26 +86,37 @@ class SysVCacheItemPool implements CacheItemPoolInterface
7086
* @type int $variableKey The variable key for getting the data from the shared memory. **Defaults to** 1.
7187
* @type string $proj The project identifier for ftok. This needs to be a one character string.
7288
* **Defaults to** 'A'.
89+
* @type string $semProj The project identifier for ftok to provide to `sem_get`. This needs to be a one
90+
* character string.
91+
* **Defaults to** 'B'.
7392
* @type int $memsize The memory size in bytes for shm_attach. **Defaults to** 10000.
7493
* @type int $perm The permission for shm_attach. **Defaults to** 0600.
7594
* }
7695
*/
7796
public function __construct($options = [])
7897
{
79-
if (! extension_loaded('sysvshm')) {
98+
if (!extension_loaded('sysvshm')) {
8099
throw new \RuntimeException(
81100
'sysvshm extension is required to use this ItemPool'
82101
);
83102
}
84103
$this->options = $options + [
85104
'variableKey' => self::VAR_KEY,
86105
'proj' => self::DEFAULT_PROJ,
106+
'semProj' => self::DEFAULT_SEM_PROJ,
87107
'memsize' => self::DEFAULT_MEMSIZE,
88108
'perm' => self::DEFAULT_PERM
89109
];
90110
$this->items = [];
91111
$this->deferredItems = [];
92112
$this->sysvKey = ftok(__FILE__, $this->options['proj']);
113+
114+
// gracefully handle when `sysvsem` isn't loaded
115+
// @TODO(v2): throw an exception when the extension isn't loaded
116+
if (extension_loaded('sysvsem')) {
117+
$semKey = ftok(__FILE__, $this->options['semProj']);
118+
$this->semId = sem_get($semKey, 1, $this->options['perm'], true);
119+
}
93120
}
94121

95122
/**
@@ -132,9 +159,17 @@ public function hasItem($key): bool
132159
*/
133160
public function clear(): bool
134161
{
162+
if (!$this->acquireLock()) {
163+
return false;
164+
}
165+
135166
$this->items = [];
136167
$this->deferredItems = [];
137-
return $this->saveCurrentItems();
168+
$ret = $this->saveCurrentItems();
169+
170+
$this->resetShm();
171+
$this->releaseLock();
172+
return $ret;
138173
}
139174

140175
/**
@@ -150,27 +185,41 @@ public function deleteItem($key): bool
150185
*/
151186
public function deleteItems(array $keys): bool
152187
{
188+
if (!$this->acquireLock()) {
189+
return false;
190+
}
191+
153192
if (!$this->hasLoadedItems) {
154193
$this->loadItems();
155194
}
156195

157196
foreach ($keys as $key) {
158197
unset($this->items[$key]);
159198
}
160-
return $this->saveCurrentItems();
199+
$ret = $this->saveCurrentItems();
200+
201+
$this->resetShm();
202+
$this->releaseLock();
203+
return $ret;
161204
}
162205

163206
/**
164207
* {@inheritdoc}
165208
*/
166209
public function save(CacheItemInterface $item): bool
167210
{
211+
if (!$this->acquireLock()) {
212+
return false;
213+
}
214+
168215
if (!$this->hasLoadedItems) {
169216
$this->loadItems();
170217
}
171218

172219
$this->items[$item->getKey()] = $item;
173-
return $this->saveCurrentItems();
220+
$ret = $this->saveCurrentItems();
221+
$this->releaseLock();
222+
return $ret;
174223
}
175224

176225
/**
@@ -187,12 +236,18 @@ public function saveDeferred(CacheItemInterface $item): bool
187236
*/
188237
public function commit(): bool
189238
{
239+
if (!$this->acquireLock()) {
240+
return false;
241+
}
242+
190243
foreach ($this->deferredItems as $item) {
191244
if ($this->save($item) === false) {
245+
$this->releaseLock();
192246
return false;
193247
}
194248
}
195249
$this->deferredItems = [];
250+
$this->releaseLock();
196251
return true;
197252
}
198253

@@ -203,20 +258,21 @@ public function commit(): bool
203258
*/
204259
private function saveCurrentItems()
205260
{
206-
$shmid = shm_attach(
207-
$this->sysvKey,
208-
$this->options['memsize'],
209-
$this->options['perm']
210-
);
211-
if ($shmid !== false) {
212-
$ret = shm_put_var(
261+
if (!$this->acquireLock()) {
262+
return false;
263+
}
264+
265+
if (false !== $shmid = $this->attachShm()) {
266+
$success = shm_put_var(
213267
$shmid,
214268
$this->options['variableKey'],
215269
$this->items
216270
);
217271
shm_detach($shmid);
218-
return $ret;
272+
$this->releaseLock();
273+
return $success;
219274
}
275+
$this->releaseLock();
220276
return false;
221277
}
222278

@@ -227,22 +283,70 @@ private function saveCurrentItems()
227283
*/
228284
private function loadItems()
229285
{
230-
$shmid = shm_attach(
231-
$this->sysvKey,
232-
$this->options['memsize'],
233-
$this->options['perm']
234-
);
235-
if ($shmid !== false) {
286+
if (!$this->acquireLock()) {
287+
return false;
288+
}
289+
290+
if (false !== $shmid = $this->attachShm()) {
236291
$data = @shm_get_var($shmid, $this->options['variableKey']);
237-
if (!empty($data)) {
238-
$this->items = $data;
239-
} else {
240-
$this->items = [];
241-
}
292+
$this->items = $data ?: [];
242293
shm_detach($shmid);
243294
$this->hasLoadedItems = true;
295+
$this->releaseLock();
296+
return true;
297+
}
298+
$this->releaseLock();
299+
return false;
300+
}
301+
302+
private function acquireLock(): bool
303+
{
304+
if ($this->semId === false) {
305+
// if `sysvsem` isn't loaded, or if `sem_get` fails, return true
306+
// this ensures BC with previous versions of the auth library.
307+
// @TODO consider better handling when `sem_get` fails.
308+
return true;
309+
}
310+
311+
$currentPid = getmypid();
312+
if ($this->lockOwnerPid === $currentPid) {
313+
// We already have the lock
314+
return true;
315+
}
316+
317+
if (sem_acquire($this->semId)) {
318+
$this->lockOwnerPid = (int) $currentPid;
244319
return true;
245320
}
246321
return false;
247322
}
323+
324+
private function releaseLock(): bool
325+
{
326+
if ($this->semId === false || $this->lockOwnerPid !== getmypid()) {
327+
return true;
328+
}
329+
330+
$this->lockOwnerPid = null;
331+
return sem_release($this->semId);
332+
}
333+
334+
private function resetShm(): void
335+
{
336+
// Remove the shared memory segment and semaphore when clearing the cache
337+
$shmid = @shm_attach($this->sysvKey);
338+
if ($shmid !== false) {
339+
@shm_remove($shmid);
340+
@shm_detach($shmid);
341+
}
342+
}
343+
344+
private function attachShm(): SysvSharedMemory|false
345+
{
346+
return shm_attach(
347+
$this->sysvKey,
348+
$this->options['memsize'],
349+
$this->options['perm']
350+
);
351+
}
248352
}

‎tests/Cache/RaceConditionTest.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ public function testRaceCondition(string $cacheClass)
4545
if (!function_exists('pcntl_fork')) {
4646
$this->markTestSkipped('pcntl_fork is not available');
4747
}
48-
for ($i = 0; $i < 100; $i++) {
49-
48+
for ($i = 0; $i < 50; $i++) {
5049
$pids = [];
5150
for ($j = 0; $j < 4; $j++) {
5251
$pid = pcntl_fork();
@@ -56,7 +55,7 @@ public function testRaceCondition(string $cacheClass)
5655
$pool = $this->createCacheItemPool($cacheClass);
5756
$item = $pool->getItem('foo');
5857
$item->set('bar');
59-
$pool->save($item);
58+
$this->assertTrue($pool->save($item));
6059

6160
if ($pid) {
6261
// parent
@@ -68,7 +67,7 @@ public function testRaceCondition(string $cacheClass)
6867
}
6968

7069
// parent
71-
$pool->save($item);
70+
$this->assertTrue($pool->save($item));
7271

7372
foreach ($pids as $pid) {
7473
pcntl_waitpid($pid, $status);
@@ -78,6 +77,8 @@ public function testRaceCondition(string $cacheClass)
7877
$this->assertTrue($pool->hasItem('foo'));
7978
$cachedItem = $pool->getItem('foo');
8079
$this->assertEquals('bar', $cachedItem->get());
80+
81+
$pool->clear();
8182
}
8283
}
8384

‎tests/Cache/SysVCacheItemPoolTest.php‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323

2424
class SysVCacheItemPoolTest extends BaseTest
2525
{
26+
const VARIABLE_KEY = 99;
27+
2628
private $pool;
2729

2830
public function setUp(): void
@@ -32,10 +34,17 @@ public function setUp(): void
3234
'sysvshm extension is required for running the test'
3335
);
3436
}
35-
$this->pool = new SysVCacheItemPool(['variableKey' => 99]);
37+
$this->pool = new SysVCacheItemPool(['variableKey' => self::VARIABLE_KEY]);
3638
$this->pool->clear();
3739
}
3840

41+
public function tearDown(): void
42+
{
43+
if (extension_loaded('sysvshm')) {
44+
$this->pool->clear();
45+
}
46+
}
47+
3948
public function saveItem($key, $value)
4049
{
4150
$item = $this->pool->getItem($key);
@@ -158,4 +167,39 @@ public function testCommitsDeferredItems()
158167
$this->pool->getItem($keys[1])->get()
159168
);
160169
}
170+
171+
public function testRaceCondition()
172+
{
173+
if (!extension_loaded('sysvsem')) {
174+
$this->markTestSkipped(
175+
'sysvsem extension is required for running the race condition test'
176+
);
177+
}
178+
179+
$key = 'race-item';
180+
$initialValue = 0;
181+
$this->saveItem($key, $initialValue);
182+
183+
$numProcesses = 100;
184+
$processes = [];
185+
for ($i = 0; $i < $numProcesses; $i++) {
186+
$command = sprintf(
187+
'php %s/sysv_cache_race_condition_writer.php %s %s',
188+
__DIR__,
189+
$key,
190+
self::VARIABLE_KEY
191+
);
192+
$processes[] = proc_open($command, [], $pipes);
193+
}
194+
195+
foreach ($processes as $process) {
196+
// proc_close waits for the process to terminate and returns its exit code.
197+
// This ensures that all child processes have completed their writes
198+
// before the parent process proceeds to read the final value.
199+
proc_close($process);
200+
}
201+
202+
$finalValue = $this->pool->getItem($key)->get();
203+
$this->assertEquals($numProcesses, $finalValue);
204+
}
161205
}

0 commit comments

Comments
 (0)