Fix for issue 6048 #6049

Merged
merged 5 commits into from Apr 14, 2014

4 participants

@brettmc

Allow ExceptionHandler plugin to suppress exceptions from Filesystem::clearExpired, by having FileSystem::clearExpired trigger an exception event that ExceptionHandler will listen for.

@brettmc brettmc Fix for issue 6048, where an apparent race condition occurs when conc…
…urrent filesystem caches try to clear expired items. To avoid this causing an exception for a user session, arrange for Filesystem::clearExpired to trigger a 'clearExpired.exception' event. This should give the same functionality as currently exists. Also, have the ExceptionHandler plugin listen for the clearExpired.exception event, and suppress it as per other exceptions.
be5228f
@Ocramius Ocramius and 1 other commented on an outdated diff Mar 27, 2014
library/Zend/Cache/Storage/Adapter/Filesystem.php
@@ -161,7 +163,10 @@ public function clearExpired()
}
$error = ErrorHandler::stop();
if ($error) {
- throw new Exception\RuntimeException("Failed to clear expired items", 0, $error);
+ $e = new Exception\RuntimeException("Failed to clear expired items", 0, $error);
+ $result = false;
+ $args = new \ArrayObject();
@Ocramius
Zend Framework member

No need to define all these parameters as variables before executing $this->triggerException()

@brettmc
brettmc added a note Mar 27, 2014

It looks a little unwieldly, but is this preferable?

return $this->triggerException(__FUNCTION__, new \ArrayObject(), $result, new Exception\RuntimeException("Failed to clear expired items", 0, $error));

I've left $result as a variable as it's passed by reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik and 1 other commented on an outdated diff Mar 27, 2014
...endTest/Cache/Storage/Plugin/ExceptionHandlerTest.php
@@ -71,6 +71,8 @@ public function testAddPlugin()
'decrementItem.exception' => 'onException',
'decrementItems.exception' => 'onException',
+

I think it is ok to remove this blank new line

@brettmc
brettmc added a note Mar 27, 2014

the surrounding code in this test groups the listeners with a blank line between, which is the style I was trying to conform to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@brettmc brettmc clean up whitespace which caused php_cs to fail the build. Adjust Fil…
…eSystem::clearExpired method per feedback from initial pull request
c251b50
@Ocramius
Zend Framework member

@brettmc this one needs a test that validates the fix

@Ocramius Ocramius added Cache bug labels Apr 2, 2014
@Ocramius
Zend Framework member

Related to #6048

@marc-mabe
Zend Framework member

looks good to me 👍

brettmc added some commits Apr 7, 2014
@brettmc brettmc add unit test for clearExpired exception. By setting the cache dir to…
… read-only, unlink() will fail, causing the exception event to be triggered.

Ensure that an empty ArrayObject is passed to triggerException - null does not work
ee317eb
@brettmc brettmc php_cs fixes 78b3673
@brettmc brettmc set exceptionHandler plugin to not throw exceptions, to prove that th…
…e exception event is handled as expected.

Add use statements for plugin and options.
d73d9fe
@weierophinney weierophinney added a commit that referenced this pull request Apr 14, 2014
@weierophinney weierophinney [#6049] CS fixes
- single instead of double quotes
- space following conditional declaration
e4a9c65
@weierophinney weierophinney added a commit that referenced this pull request Apr 14, 2014
@weierophinney weierophinney Merge branch 'hotfix/6049' into develop
Forward port #6049
ce50ec6
@weierophinney weierophinney added this to the 2.3.1 milestone Apr 14, 2014
@weierophinney weierophinney merged commit d73d9fe into zendframework:master Apr 14, 2014

1 check passed

Details continuous-integration/travis-ci The Travis CI build passed
@weierophinney weierophinney added a commit that referenced this pull request Apr 14, 2014
@weierophinney weierophinney Merge branch 'hotfix/6049'
Close #6049
Fixes #6048
d16d3a3
@weierophinney weierophinney self-assigned this Apr 14, 2014
@brettmc brettmc deleted the unknown repository branch Apr 15, 2014
@weierophinney weierophinney added a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#6049 from brettmc/6048
Fix for issue 6048
abd1083
@weierophinney weierophinney added a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
@weierophinney weierophinney [zendframework/zendframework#6049] CS fixes
- single instead of double quotes
- space following conditional declaration
6a91c34
@weierophinney weierophinney added a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/6049' 86a32e6
@weierophinney weierophinney added a commit to zendframework/zend-cache that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge branch 'hotfix/6049' into develop 3bcbe8e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment