Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Commit

Permalink
Merge branch 'hotfix/102', Close #102
Browse files Browse the repository at this point in the history
  • Loading branch information
@marc-mabe
marc-mabe committed Jun 25, 2016
2 parents d93aaa2 + 03ba74c commit 1f6d041
Show file tree
Hide file tree
Showing 4 changed files with 254 additions and 33 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ All notable changes to this project will be documented in this file, in reverse

### Fixed

- Nothing.
- [#102](https://github.com/zendframework/zend-cache/pull/102)
filesystem: fixes a lot of possible race conditions

## 2.7.1 - 2016-05-12

Expand Down
76 changes: 62 additions & 14 deletions src/Storage/Adapter/Filesystem.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,14 @@ public function flush()
$clearFolder($pathname);
rmdir($pathname);
} else {
// remove the file by ignoring errors if the file doesn't exist afterwards
// to fix a possible race condition if onother process removed the faile already.
ErrorHandler::start();
unlink($pathname);
$err = ErrorHandler::stop();
if ($err && file_exists($pathname)) {
ErrorHandler::addError($err->getSeverity(), $err->getMessage(), $err->getFile(), $err->getLine());
}
}
}
};
Expand Down Expand Up @@ -140,7 +147,7 @@ public function clearExpired()
$namespace = $options->getNamespace();
$prefix = ($namespace === '') ? '' : $namespace . $options->getNamespaceSeparator();

$flags = GlobIterator::SKIP_DOTS | GlobIterator::CURRENT_AS_FILEINFO;
$flags = GlobIterator::SKIP_DOTS | GlobIterator::CURRENT_AS_PATHNAME;
$path = $options->getCacheDir()
. str_repeat(DIRECTORY_SEPARATOR . $prefix . '*', $options->getDirLevel())
. DIRECTORY_SEPARATOR . $prefix . '*.dat';
Expand All @@ -149,15 +156,30 @@ public function clearExpired()
$ttl = $options->getTtl();

ErrorHandler::start();
foreach ($glob as $entry) {
$mtime = $entry->getMTime();
if ($time >= $mtime + $ttl) {
$pathname = $entry->getPathname();
foreach ($glob as $pathname) {
// get last modification time of the file but ignore if the file is missing
// to fix a possible race condition if onother process removed the faile already.
ErrorHandler::start();
$mtime = filemtime($pathname);
$err = ErrorHandler::stop();
if ($err && file_exists($pathname)) {
ErrorHandler::addError($err->getSeverity(), $err->getMessage(), $err->getFile(), $err->getLine());
} elseif ($time >= $mtime + $ttl) {
// remove the file by ignoring errors if the file doesn't exist afterwards
// to fix a possible race condition if onother process removed the faile already.
ErrorHandler::start();
unlink($pathname);

$tagPathname = substr($pathname, 0, -4) . '.tag';
if (file_exists($tagPathname)) {
$err = ErrorHandler::stop();
if ($err && file_exists($pathname)) {
ErrorHandler::addError($err->getSeverity(), $err->getMessage(), $err->getFile(), $err->getLine());
} else {
$tagPathname = substr($pathname, 0, -4) . '.tag';
ErrorHandler::start();
unlink($tagPathname);
$err = ErrorHandler::stop();
if ($err && file_exists($pathname)) {
ErrorHandler::addError($err->getSeverity(), $err->getMessage(), $err->getFile(), $err->getLine());
}
}
}
}
Expand Down Expand Up @@ -202,11 +224,24 @@ public function clearByNamespace($namespace)

ErrorHandler::start();
foreach ($glob as $pathname) {
// remove the file by ignoring errors if the file doesn't exist afterwards
// to fix a possible race condition if onother process removed the faile already.
ErrorHandler::start();
unlink($pathname);
$err = ErrorHandler::stop();
if ($err && file_exists($pathname)) {
ErrorHandler::addError($err->getSeverity(), $err->getMessage(), $err->getFile(), $err->getLine());
}
}
$error = ErrorHandler::stop();
if ($error) {
throw new Exception\RuntimeException("Failed to remove files of '{$path}'", 0, $error);
$err = ErrorHandler::stop();
if ($err) {
$result = false;
return $this->triggerException(
__FUNCTION__,
new ArrayObject(),
$result,
new Exception\RuntimeException("Failed to clear items of namespace '{$namespace}'", 0, $err)
);
}

return true;
Expand Down Expand Up @@ -240,11 +275,24 @@ public function clearByPrefix($prefix)

ErrorHandler::start();
foreach ($glob as $pathname) {
// remove the file by ignoring errors if the file doesn't exist afterwards
// to fix a possible race condition if onother process removed the faile already.
ErrorHandler::start();
unlink($pathname);
$err = ErrorHandler::stop();
if ($err && file_exists($pathname)) {
ErrorHandler::addError($err->getSeverity(), $err->getMessage(), $err->getFile(), $err->getLine());
}
}
$error = ErrorHandler::stop();
if ($error) {
throw new Exception\RuntimeException("Failed to remove files of '{$path}'", 0, $error);
$err = ErrorHandler::stop();
if ($err) {
$result = false;
return $this->triggerException(
__FUNCTION__,
new ArrayObject(),
$result,
new Exception\RuntimeException("Failed to remove files of '{$path}'", 0, $err)
);
}

return true;
Expand Down
200 changes: 186 additions & 14 deletions test/Storage/Adapter/FilesystemTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,15 +346,18 @@ public function testClearByPrefixWithUnexpectedDirectory()
/**
* @runInSeparateProcess
*/
public function testClearByTagsWithoutLocking()
public function testRaceConditionInClearByTags()
{
if (!function_exists('pcntl_fork') || !function_exists('posix_kill')) {
$this->markTestSkipped('Missing pcntl_fork and/or posix_kill');
}

// delay unlink() by global variable $unlinkDelay
global $unlinkDelay;
require __DIR__ . '/TestAsset/FilesystemDelayedUnlink.php';

// create cache items
$this->_storage->getOptions()->setDirLevel(0);
$this->_storage->getOptions()->setFileLocking(false);
$this->_storage->setItems([
'a_key' => 'a_value',
'b_key' => 'b_value',
Expand All @@ -370,14 +373,15 @@ public function testClearByTagsWithoutLocking()
// The parent process
// Slow down unlink function and start removing items.
// Finally test if the item not matching the tag was removed by the child process.
require __DIR__ . '/TestAsset/FilesystemDelayedUnlink.php';
$unlinkDelay = 5000;

$this->_storage->clearByTags(['a_tag'], true);
$this->assertFalse($this->_storage->hasItem('other'));
$this->assertFalse($this->_storage->hasItem('other'), 'Child process does not run as expected');
} else {
// The child process:
// Wait to make sure the parent process has started determining files to unlink.
// Than remove one of the items the parent process should remove and another item for testing.
usleep(10000);
usleep(1000);
$this->_storage->removeItems(['b_key', 'other']);
posix_kill(posix_getpid(), SIGTERM);
}
Expand All @@ -386,22 +390,138 @@ public function testClearByTagsWithoutLocking()
/**
* @runInSeparateProcess
*/
public function testClearByTagsWithLocking()
public function testRaceConditionInClearByNamespace()
{
if (!function_exists('pcntl_fork') || !function_exists('posix_kill')) {
$this->markTestSkipped('Missing pcntl_fork and/or posix_kill');
}

// delay unlink() by global variable $unlinkDelay
global $unlinkDelay;
require __DIR__ . '/TestAsset/FilesystemDelayedUnlink.php';

// create cache items
$this->_storage->getOptions()->setDirLevel(0);
$this->_storage->getOptions()->setNamespace('ns-other');
$this->_storage->setItems([
'other' => 'other',
]);
$this->_storage->getOptions()->setNamespace('ns-4-clear');
$this->_storage->setItems([
'a_key' => 'a_value',
'b_key' => 'b_value',
]);

$pidChild = pcntl_fork();
if ($pidChild == -1) {
$this->fail('pcntl_fork() failed');
} elseif ($pidChild) {
// The parent process
// Slow down unlink function and start removing items.
// Finally test if the item not matching the tag was removed by the child process.
$unlinkDelay = 5000;

$this->_storage->getOptions()->setNamespace('ns-4-clear');
$this->_storage->clearByNamespace('ns-4-clear');

$this->assertFalse($this->_storage->hasItem('a_key'));
$this->assertFalse($this->_storage->hasItem('b_key'));

$this->_storage->getOptions()->setNamespace('ns-other');
$this->assertFalse($this->_storage->hasItem('other'), 'Child process does not run as expected');
} else {
// The child process:
// Wait to make sure the parent process has started determining files to unlink.
// Than remove one of the items the parent process should remove and another item for testing.
usleep(1000);

$this->_storage->getOptions()->setNamespace('ns-4-clear');
$this->assertTrue($this->_storage->removeItem('b_key'));

$this->_storage->getOptions()->setNamespace('ns-other');
$this->assertTrue($this->_storage->removeItem('other'));

posix_kill(posix_getpid(), SIGTERM);
}
}

/**
* @runInSeparateProcess
*/
public function testRaceConditionInClearByPrefix()
{
if (!function_exists('pcntl_fork') || !function_exists('posix_kill')) {
$this->markTestSkipped('Missing pcntl_fork and/or posix_kill');
}

// delay unlink() by global variable $unlinkDelay
global $unlinkDelay;
require __DIR__ . '/TestAsset/FilesystemDelayedUnlink.php';

// create cache items
$this->_storage->getOptions()->setDirLevel(0);
$this->_storage->getOptions()->setNamespace('ns');
$this->_storage->setItems([
'prefix_a_key' => 'a_value',
'prefix_b_key' => 'b_value',
'other' => 'other',
]);

$pidChild = pcntl_fork();
if ($pidChild == -1) {
$this->fail('pcntl_fork() failed');
} elseif ($pidChild) {
// The parent process
// Slow down unlink function and start removing items.
// Finally test if the item not matching the tag was removed by the child process.
$unlinkDelay = 5000;

$this->_storage->clearByPrefix('prefix_');

$this->assertFalse($this->_storage->hasItem('prefix_a_key'));
$this->assertFalse($this->_storage->hasItem('prefix_b_key'));

$this->assertFalse($this->_storage->hasItem('other'), 'Child process does not run as expected');
} else {
// The child process:
// Wait to make sure the parent process has started determining files to unlink.
// Than remove one of the items the parent process should remove and another item for testing.
usleep(1000);

$this->assertTrue($this->_storage->removeItem('prefix_b_key'));
$this->assertTrue($this->_storage->removeItem('other'));

posix_kill(posix_getpid(), SIGTERM);
}
}

/**
* @runInSeparateProcess
*/
public function testRaceConditionInClearExpired()
{
if (!function_exists('pcntl_fork') || !function_exists('posix_kill')) {
$this->markTestSkipped('Missing pcntl_fork and/or posix_kill');
}

// delay unlink() by global variable $unlinkDelay
global $unlinkDelay;
require __DIR__ . '/TestAsset/FilesystemDelayedUnlink.php';

// create cache items
$this->_storage->getOptions()->setDirLevel(0);
$this->_storage->getOptions()->setFileLocking(true);
$this->_storage->getOptions()->setTtl(2);
$this->_storage->setItems([
'a_key' => 'a_value',
'b_key' => 'b_value',
'other' => 'other',
]);
$this->_storage->setTags('a_key', ['a_tag']);
$this->_storage->setTags('b_key', ['a_tag']);

// wait TTL seconds and touch item other so this item will not be deleted by clearExpired
// and can be used for testing the child process
$this->waitForFullSecond();
sleep(2);
$this->_storage->touchItem('other');

$pidChild = pcntl_fork();
if ($pidChild == -1) {
Expand All @@ -410,15 +530,67 @@ public function testClearByTagsWithLocking()
// The parent process
// Slow down unlink function and start removing items.
// Finally test if the item not matching the tag was removed by the child process.
require __DIR__ . '/TestAsset/FilesystemDelayedUnlink.php';
$this->_storage->clearByTags(['a_tag'], true);
$this->assertFalse($this->_storage->hasItem('other'));
$unlinkDelay = 5000;

$this->_storage->clearExpired();

$this->assertFalse($this->_storage->hasItem('a_key'));
$this->assertFalse($this->_storage->hasItem('b_key'));

$this->assertFalse($this->_storage->hasItem('other'), 'Child process does not run as expected');
} else {
// The child process:
// Wait to make sure the parent process has started determining files to unlink.
// Than remove one of the items the parent process should remove and another item for testing.
usleep(10000);
$this->_storage->removeItems(['b_key', 'other']);
usleep(1000);

$this->assertTrue($this->_storage->removeItem('b_key'));
$this->assertTrue($this->_storage->removeItem('other'));

posix_kill(posix_getpid(), SIGTERM);
}
}

/**
* @runInSeparateProcess
*/
public function testRaceConditionInFlush()
{
if (!function_exists('pcntl_fork') || !function_exists('posix_kill')) {
$this->markTestSkipped('Missing pcntl_fork and/or posix_kill');
}

// delay unlink() by global variable $unlinkDelay
global $unlinkDelay;
require __DIR__ . '/TestAsset/FilesystemDelayedUnlink.php';

// create cache items
$this->_storage->getOptions()->setDirLevel(0);
$this->_storage->setItems([
'a_key' => 'a_value',
'b_key' => 'b_value',
]);

$pidChild = pcntl_fork();
if ($pidChild == -1) {
$this->fail('pcntl_fork() failed');
} elseif ($pidChild) {
// The parent process
// Slow down unlink function and start removing items.
$unlinkDelay = 5000;

$this->_storage->flush();

$this->assertFalse($this->_storage->hasItem('a_key'));
$this->assertFalse($this->_storage->hasItem('b_key'));
} else {
// The child process:
// Wait to make sure the parent process has started determining files to unlink.
// Than remove one of the items the parent process should remove.
usleep(1000);

$this->assertTrue($this->_storage->removeItem('b_key'));

posix_kill(posix_getpid(), SIGTERM);
}
}
Expand Down
8 changes: 4 additions & 4 deletions test/Storage/Adapter/TestAsset/FilesystemDelayedUnlink.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

function unlink($path, $context = null)
{
usleep(50000);
if ($context) {
return \unlink($path, $context);
global $unlinkDelay;
if (isset($unlinkDelay) && $unlinkDelay > 0) {
usleep($unlinkDelay);
}

return \unlink($path);
return $context ? \unlink($path, $context) : \unlink($path);
}

0 comments on commit 1f6d041

Please sign in to comment.