Survive a class file list that has gone stale, and stop making one - #1231
Merged
Conversation
An install rm -rf's the web root and rebuilds it, so a plugin release that
DROPS a file really removes it -- fog-plugins v1.6.11 drops
ldap/hooks/addldapapi.hook.php. Initiator::classFileList() caches the
scanned class-file list for 300 seconds and does not know the tree just
changed underneath it, so every request in what is left of that window
walks a list naming a file that is gone.
startClassFromFiles() died on the first one. include_once found nothing,
getClass()'s ReflectionClass threw, and nothing catches it -- inside
EventManager::load(), inside LoadGlobals, which is every entry point. The
result is a bodyless 500 on the whole site, including the installer's own
"Checking web server serves FOG" probe a few steps later, which then
reports a failed install against a server that is fine. It heals when the
TTL expires, so it reads as a flaky install rather than as this.
Staleness is designed in. forgetClassFileList() exists precisely because
the cache can describe a tree that has since changed, and says so in its
docblock -- it is just never called by the installer, only by the plugin
uploader. So every consumer has to tolerate a stale list, and this was the
one that treated it as fatal.
Two guards, because either alone leaves a hole:
the installer drops $fogprogramdir/cache/filelist.*.json AFTER replacing
the web root, so the window does not exist. Ordering is the whole content
of that fix -- dropping it first just rebuilds it from the old tree. The
file lists only: that directory also carries the settings-cache flush
signal.
startClassFromFiles() skips a vanished file, and catches
ReflectionException around getClass() for the other cause -- a file that
is present but does not declare the class its name promises. Covers a
stale list from any source: an admin deleting a plugin by hand, an NFS
lag, a half-finished copy.
Reported through error_log(), not self::error(). _writeLog() is gated on
`self::$mySchema >= FOG_SCHEMA` and a globalSettings read, and this path
runs inside LoadGlobals during an install -- which is exactly when that
gate is closed, so the report would be dropped in the only situation it
fires in. The PHP error log is also where the fatal it replaces appeared,
so both lines land in one place. Precedent: authorization.class.php,
hostmanager.class.php.
Skipped and reported rather than swallowed: a listener that does not load
is a feature that silently stops happening.
Verified against a real stale cache, the same cache file either side, the
whole experiment inside the TTL:
cache names the deleted file: 1
PRE-FIX PHP Fatal error: Uncaught ReflectionException:
Class "addldapapi" does not exist ... fogbase.class.php:588
POST-FIX FOG startClassFromFiles: ... no longer exists; skipping
addldapapi. BOOTED OK
Five mutations checked, all caught: removing the is_file guard, removing
the ReflectionException catch, skipping silently, the installer not
dropping the cache, and the installer wiping the whole cache directory
instead of the file lists.
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found live:
installfog.shreportedon a server that was fine.
What happens
An install
rm -rfs the web root and rebuilds it, so a plugin release that drops a file really removes it —fog-pluginsv1.6.11 dropsldap/hooks/addldapapi.hook.php.Initiator::classFileList()caches the scanned file list for 300 seconds and does not know the tree changed underneath it, so every request in the rest of that window walks a list naming a file that is gone.startClassFromFiles()died on the first one, uncaught, insideLoadGlobals— which is every entry point. Bodyless 500 on the whole site, including the installer's own probe a few steps later. It heals when the TTL expires, which is why it reads as a flaky install rather than as a bug.Staleness is designed in.
forgetClassFileList()exists precisely because the cache can describe a tree that has since changed, and its docblock says so — it is just never called by the installer, only by the plugin uploader. Every consumer has to tolerate a stale list; this was the one that treated it as fatal.Two guards, because either alone leaves a hole
$fogprogramdir/cache/filelist.*.jsonafter replacing the web root, so the window does not exist. Ordering is the whole content of the fix — dropping it first just rebuilds it from the old tree. File lists only: that directory also carries the settings-cache flush signal.startClassFromFiles()skips a vanished file, and catchesReflectionExceptionaroundgetClass()for the other cause — a file present that does not declare the class its name promises. Covers staleness from any source: an admin deleting a plugin by hand, NFS lag, a half-finished copy.Reported through
error_log(), notself::error():_writeLog()is gated onself::$mySchema >= FOG_SCHEMAand aglobalSettingsread, and this path runs insideLoadGlobalsduring an install — exactly when that gate is closed. It would be dropped in the only situation it fires in. The PHP error log is also where the fatal it replaces appeared, so both lines land in one place.Skipped and reported, not swallowed — a listener that does not load is a feature that silently stops happening.
Verification
Against a real stale cache, the same cache file either side, whole experiment inside the TTL:
tests/stale-class-file-list.test.php— 8 checks. Behavioural for the loader (a real file created, deleted, then passed tostartClassFromFiles(), asserting it returns and logs); source-level for the installer half, which needs a live install to run, and the comment says which is which rather than dressing one up as the other.Five mutations checked, all caught: removing the
is_fileguard, removing theReflectionExceptioncatch, skipping silently, the installer not dropping the cache, and the installer wiping the whole cache directory instead of just the file lists.Full suite: 76 passed, 0 failed.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GN7hADN5QLzoxXWDA6xeUk