Merge branch 'hotfix/44', Close #44 and PR #82

zendframework · Mar 13, 2016 · bb8a75c · bb8a75c
2 parents f6e4727 + 1752534
commit bb8a75c
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -19,6 +19,8 @@ All notable changes to this project will be documented in this file, in reverse
 
 ### Fixed
 
+- [#44](https://github.com/zendframework/zend-cache/issues/44)
+  Filesystem: fixed race condition in method clearByTags
 - [#59](https://github.com/zendframework/zend-cache/pull/59)
   XCache: fixed broken internalSetItem() with empty namespace
 - [#58](https://github.com/zendframework/zend-cache/issues/58)

diff --git a/src/Storage/Adapter/Filesystem.php b/src/Storage/Adapter/Filesystem.php
@@ -328,7 +328,16 @@ public function clearByTags(array $tags, $disjunction = false)
         $glob = new GlobIterator($path, $flags);
 
         foreach ($glob as $pathname) {
-            $diff = array_diff($tags, explode("\n", $this->getFileContent($pathname)));
+            try {
+                $diff = array_diff($tags, explode("\n", $this->getFileContent($pathname)));
+            } catch (Exception\RuntimeException $exception) {
+                // ignore missing files because of possible raise conditions
+                // e.g. another process already deleted that item
+                if (!file_exists($pathname)) {
+                    continue;
+                }
+                throw $exception;
+            }
 
             $rem  = false;
             if ($disjunction && count($diff) < $tagCount) {

diff --git a/test/Storage/Adapter/FilesystemTest.php b/test/Storage/Adapter/FilesystemTest.php
@@ -328,4 +328,84 @@ public function testClearByPrefixWithUnexpectedDirectory()
         $prefix = substr(md5('a_key'), 2, 2);
         $this->_storage->clearByPrefix($prefix);
     }
+
+    /**
+     * @runInSeparateProcess
+     */
+    public function testClearByTagsWithoutLocking()
+    {
+        if (!function_exists('pcntl_fork') || !function_exists('posix_kill')) {
+            $this->markTestSkipped('Missing pcntl_fork and/or posix_kill');
+        }
+
+        // create cache items
+        $this->_storage->getOptions()->setDirLevel(0);
+        $this->_storage->getOptions()->setFileLocking(false);
+        $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']);
+
+        $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.
+            require __DIR__ . '/TestAsset/FilesystemDelayedUnlink.php';
+            $this->_storage->clearByTags(['a_tag'], true);
+            $this->assertFalse($this->_storage->hasItem('other'));
+        } 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']);
+            posix_kill(posix_getpid(), SIGTERM);
+        }
+    }
+
+    /**
+     * @runInSeparateProcess
+     */
+    public function testClearByTagsWithLocking()
+    {
+        if (!function_exists('pcntl_fork') || !function_exists('posix_kill')) {
+            $this->markTestSkipped('Missing pcntl_fork and/or posix_kill');
+        }
+
+        // create cache items
+        $this->_storage->getOptions()->setDirLevel(0);
+        $this->_storage->getOptions()->setFileLocking(true);
+        $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']);
+
+        $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.
+            require __DIR__ . '/TestAsset/FilesystemDelayedUnlink.php';
+            $this->_storage->clearByTags(['a_tag'], true);
+            $this->assertFalse($this->_storage->hasItem('other'));
+        } 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']);
+            posix_kill(posix_getpid(), SIGTERM);
+        }
+    }
 }
diff --git a/test/Storage/Adapter/TestAsset/FilesystemDelayedUnlink.php b/test/Storage/Adapter/TestAsset/FilesystemDelayedUnlink.php
@@ -0,0 +1,13 @@
+<?php
+
+namespace Zend\Cache\Storage\Adapter;
+
+function unlink($path, $context = null)
+{
+    usleep(50000);
+    if ($context) {
+        return \unlink($path, $context);
+    }
+
+    return \unlink($path);
+}