cache: reopen remote directory handle on cache-miss readdir#375
Open
paulojeronimo wants to merge 1 commit into
Open
cache: reopen remote directory handle on cache-miss readdir#375paulojeronimo wants to merge 1 commit into
paulojeronimo wants to merge 1 commit into
Conversation
cache_readdir() reused the already-open remote directory handle whenever the cached listing was missing or invalidated, instead of just when it was still valid. The SFTP directory stream is positional: once read to EOF, rereading the same handle after a local create/delete/rename returns EOF again, making the directory look empty even though its contents changed on the server. Close and reopen the handle before the live read whenever we fall through the cache-hit fast path, so the read reflects the current directory instead of a stale, exhausted stream.
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.
Problem
After any write through an sshfs mount (create, delete, rename), a subsequent directory listing (
ls,readdir()in general) on the affected directory can come back empty, even though the remote directory and its contents are completely intact. Only the listing is wrong -stat()/open()/read()on a specific, known file path inside the "empty" directory still work and return correct data. Unmounting and remounting always restores a correct listing until the next write.Reproduction:
Confirmed via an
sshfs -f -dprotocol trace: a singleOPENDIRrequest was reused to serve multiplelsinvocations across atouch+rmcycle, with noCLOSEDIR/reopen in between. The post-writeREADDIRrequest came back as an immediateSTATUS(EOF) instead of aNAMEreply, because the remote SFTP directory stream behind that handle had already been read to EOF by the earlier listing.Root cause
cache_readdir()incache.cinvalidates the directory cache correctly on write (cache_invalidate_dir()), but its refill path reuses the handle unconditionally, regardless of whether it got there through a valid cache hit or a cache miss:The remote SFTP directory stream is positional/stateful. Once a
file_handlehas been read to EOF by an earliercache_readdir()call, reusing it for a later "live" read (triggered by cache invalidation from a write) just returns EOF again - the directory looks empty regardless of what actually changed on the server.This is not a recent regression: the same logic is present in the current
3.7.6release and inmasteras of this writing (the only post-3.7.6 changes tocache.care unrelated: filling stat info from cached readdir results, and a memleak fix in readlink).Related to #211 ("Seeking in directory stream has no effect"), which describes the same user-visible symptom (directory appears empty after create/delete cycles through
mc) and was closed as a duplicate in 2020. It does not appear to have ever been fixed - the handle-reuse-without-reopen behavior it points at is still incache_readdir()today.Fix
When falling through to a live read because the cache was missing or invalidated, close the existing remote handle (if any) and open a fresh one before reading, instead of reusing a stream that may already be exhausted:
The existing valid-cache-hit fast path (serving
node->dirdirectly) returns earlier in the function and is unaffected - the extrareleasedir+opendirround trip only happens on the cache-miss/refresh path, which is exactly the case that was broken. No new mount option, no behavior change when the cache is warm.Testing
meson setup build && ninja -C buildagainst macFUSE/glib2/openssh (macOS).ls,lsagain (cache hit, correct),touch+rm,lsagain (empty - bug reproduced).-f -d) that the fix path is actually exercised: after each write-triggered invalidation, the log now showsCLOSEon the directory handle followed by a freshOPENDIR, where the unpatched binary showed the old handle reused with noCLOSEat all.sshfs(macOS/macFUSE) and confirmed the fix holds on a real, long-lived mount used for day-to-day file sharing.