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

Bugfix/clear by tag on filesystem storage while files are already deleted #82

Conversation

skors
Copy link
Contributor

@skors skors commented Feb 19, 2016

This PR fixes the behavior on the special case that file content is read while tag file is deleted

More description in #44

@marc-mabe is this kind of try...catch solution you had in mind?

Merging this PR closes #44

the unlink method in Zend\Cache\Storage\Adapter namespace is overloaded
to insert some delay on deletion process.
this tests deletion of tag files while deleting by tags is in progress
in another process
while looping through directory iterator, file skip files which cannot
be opened
}

throw $exception;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very error prone. I think it would be better to test if the file exists instead of parsing an exception message.

try {
    // ...
} catch (Exception\RuntimeException $e) {
    // ignore missing files because of possible raise conditions
    // e.g. another process already deleted that item
    if (!file_exists($pathname)) {
        continue;
    }
    throw $e;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DCMNMarc thanks, that was unclear to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DCMNMarc i've changed it. but wouldn't this behave same like you commented in #44 (comment) ?

@marc-mabe marc-mabe self-assigned this Mar 2, 2016
@marc-mabe marc-mabe added the bug label Mar 2, 2016
@marc-mabe
Copy link
Member

@skors Much thanks for this fix and unittest!

Today I had time play a little with it and was able to reduce the wait timings and removed the temporary file the tests still works (or fails) as expected.

This way the test should also fail if the process wait timings are not proper or even if something changed in the filesystem adapter which breaks the hack for delay unlink.

But I still doesn't understand why posix_kill(posix_getpid(), SIGTERM); is required.

Now it looks like the following:

    /**
     * @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);
        }
    }

@marc-mabe
Copy link
Member

closed by bb8a75c

Thanks!

@marc-mabe marc-mabe closed this Mar 13, 2016
@marc-mabe marc-mabe added this to the 2.7.0 milestone Apr 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clearByTags in Filesystem adapter crashes when .tag file deleted during GlobIterator loop
2 participants