fix(LocalStorage): Clear realpath cache on fopen failure and re-try access - #63101
fix(LocalStorage): Clear realpath cache on fopen failure and re-try access#63101DerDreschner wants to merge 1 commit into
Conversation
|
/backport to stable34 |
|
/backport to stable33 |
|
/backport to stable32 |
|
/backport to stable31 |
|
/backport to stable30 |
|
/backport to stable29 |
|
Backport down to stable29 is to stabilize our CI runs. |
joshtrichards
left a comment
joshtrichards
left a comment
There was a problem hiding this comment.
Some of the comments may be better represented by (or at least in) the regression tests. 🤷
| // realpath_cache_ttl expires, this process still has it cached as a file, and | ||
| // because fopen() resolves through that cache, it refuses to look inside and | ||
| // fails as if the file did not exist. Dropping the stale entries and opening | ||
| // again is what fixes that. The retry deliberately stays unsuppressed, so the |
There was a problem hiding this comment.
I'm all for making troubleshooting clues not invisible, but this is an API contract change. Not sure...
There was a problem hiding this comment.
Hmm, kinda... The alternative would be to disable the realpath cache entirely by setting realpath_cache_size=0... At least for the test runners... But our own code relies on the current state reported by the filesystem ops to be correct, so, I think it's correct to fix it in the code itself as the issue can occur via CLI or web as well... Although I see that this is different from what PHP's own fopen() does...
| $this->unlink($path); | ||
| } | ||
| $result = @fopen($sourcePath, $mode); | ||
| if ($result === false) { |
There was a problem hiding this comment.
Any non-cache-related failures worth ignoring? (Not even sure it's possible).
There was a problem hiding this comment.
It's not possible, fopen() only returns false on any error and it's only worth looking into the error once cache is being reset and the retry happens... Which is then being logged as warning. Ignoring anything here would require the error handler to replace before/after the second fopen(), so, quite hacky.
|
+1 to an official php.net docs clarification in this area. ;-) I wander.if this is relevant for any other |
deec1b8 to
ceca32a
Compare
…ccess Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
ceca32a to
68a88bc
Compare
3 participants
Summary
PHP resolves the path of every
fopen()through its realpath cache, which stores per path component whether that component is a directory. That cache is per process, and entries live forrealpath_cache_ttl(120s by default). If one process has a path cached as a file and another process replaces it with a directory, the first process keeps refusing to descend into it andfopen()fails as if the file were missing - whilestat()/file_exists()bypass that cache and keep reporting the file. The file looks perfectly present and is still unreadable, for up to two minutes.That is why the
filesdropintegration tests fail intermittently: the suite recycles the same user and paths, one scenario storesdrop/Alice/folderas a file, the next needs it as a directory, and the dev server runs several worker processes (PHP_CLI_SERVER_WORKERS=2), so the worker serving the download is usually not the one that created the directory (example run: …). It is not CI-specific: php-fpm workers keep the same cache, so a user who deletes a fileX, creates a folderX, and downloadsX/y.txtvia another worker gets a 503 for up torealpath_cache_ttl.This is being fixed by this PR, which in this case walks up the parents, clears their cache entry and re-tries to access the path.
Checklist
3. to review, feature component)stable32)AI (if applicable)