Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpKernel] clearstatcache() so the Cache sees when a .lck file has been released#16312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
the file path should be passed into the second args:
clearstatcache(true,$path);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Oh, good catch!
jakzal commentedOct 22, 2015
From thephp.net/clearstatcache docs:
I don't see how would this be different for apache prefork MPM?
As long as this is just an assumption, and not a confirmed bug, I'm 👎 for this change. |
mpdude commentedOct 22, 2015
Thoughts:
<?phpwhile (is_file("lock")) {print"lock exists...\n";sleep(1);}
I know that these are just hints – so what kind of "evidence" or confirmation are we looking for? |
mpdude commentedOct 22, 2015
It also says:
Maybe the note regarding |
jakzal commentedOct 22, 2015
Indeed. Looks like you're right here. |
MatTheCat commentedNov 22, 2015
@mpdude it's been a month, can you confirm this PR solved the problem? |
mpdude commentedNov 22, 2015
Haven't seen it for a while, but that's not a 100% guarantee... |
fabpot commentedNov 28, 2015
Thank you@mpdude. |
…k file has been released (mpdude)This PR was squashed before being merged into the 2.3 branch (closes#16312).Discussion----------[HttpKernel] clearstatcache() so the Cache sees when a .lck file has been released| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#15813| License | MIT| Doc PR | n/aI've been trying to debug#15813 and modified the Store in a way to keep unique request IDs in the .lck file. That way, I was hoping to find out which request is blocking and/or if the request is actually still running.It turned out that `is_file()` would claim that a lock file still exists, but a subsequent attempt to read the information from that file returned "file not found" errors.So, my assumption is that the `is_file()` result is based on the fstat cache and wrong once a process has seen the lock file.@jakzal said in#15813 (comment) that `unlink()`ing the lock file should clear the statcache, but I doubt this is true across PHP processes.Commits-------982710f [HttpKernel] clearstatcache() so the Cache sees when a .lck file has been released
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is the file check absolutely necessary or couldn't we just callfile_exists() which doesn't use the stat cache?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The documentation ofclearstatcache saysfile_exists is affected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@MatTheCat is right here, only the information about non-existent files is not cached:
You should also note that PHP doesn't cache information about non-existent files. So, if you call file_exists() on a file that doesn't exist, it will return FALSE until you create the file. If you create the file, it will return TRUE even if you then delete the file. However unlink() clears the cache automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks, I think I confused that.
This PR was merged into the 2.7 branch.Discussion----------[HttpKernel] Use flock() for HttpCache's lock files| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | no| Fixed tickets |#16777,#15813 and#16312 are also related| License | MIT| Doc PR |When a PHP process crashes or terminates (maybe the OOM killer kicks in or other bad things ™️ happen) while the `HttpCache` holds a `.lck` file, that lock file may not get `unlink()`ed.The result is that other requests trying to access this cache entry will see a few seconds delay while waiting for the lock; they will eventually continue but send 503 status codes along with the response. The sudden buildup of PHP processes caused by the additional delay may cause further problems (sudden load increase).As `LockHandler` is using `flock()`-based locking, locks should be released by the OS when the PHP process terminates.I wrote this as bugfix against 2.7 because every once in a while I encounter situations (not always reproducible) where `.lock` files are left over and keep the cache locked.Commits-------2668edd [HttpKernel] Use flock() for HttpCache's lock files
I've been trying to debug#15813 and modified the Store in a way to keep unique request IDs in the .lck file. That way, I was hoping to find out which request is blocking and/or if the request is actually still running.
It turned out that
is_file()would claim that a lock file still exists, but a subsequent attempt to read the information from that file returned "file not found" errors.So, my assumption is that the
is_file()result is based on the fstat cache and wrong once a process has seen the lock file.@jakzal said in#15813 (comment) that
unlink()ing the lock file should clear the statcache, but I doubt this is true across PHP processes.