KickCleaner sets the out of space flag, kicks the cleaner and then waits for
that flag to clear. Only the cleaner clears it, and the cleaner returns
immediately when the cache poll interval is not positive, so when it is
disabled nothing ever reads the kick or clears the flag and the caller waits
forever.
It now returns straight away in that case, under the same condition the
cleaner itself uses to decide it is disabled. Callers already retry a bounded
number of times and then report the error, which is the right outcome when
nothing is going to free space.
Write() overwrote a successful write's nil error with the stale
lastErr returned by kickWaiters() once the downloader had recorded
too many errors. download() then wrapped that stale error again and
stored it back as the new lastErr, so every subsequent write added
another "vfs reader: failed to write to cache file:" prefix - fixes#4998
_dispatchWaiters decided whether a waiter was satisfied by clipping its
range against dls.src.Size(), the size of the fs.Object snapshot taken
when the Downloaders was created. _ensureDownloader decided whether to
start a downloader from Item.FindMissing, which clips against
item.info.Size instead.
When item.info.Size dropped below the offset a waiter was parked on while
the source object still reported the full size, the two disagreed.
_ensureDownloader found nothing missing so it started no downloader, and
_dispatchWaiters found the range absent so it never released the waiter.
Nothing was downloaded and no error was produced, so the error count never
reached maxErrorCount and the waiter was never woken. The reader blocked
forever with nothing logged at any level.
Wake a waiter when FindMissing reports nothing left to download for it as
well as when its data has arrived. Since _ensureDownloader starts a
downloader only when FindMissing is non empty, a waiter with nothing
missing has nothing that could ever wake it.
Fixes#9769
The VFS calls backends from goroutines of its own. A panic on any of these
cannot be recovered. So a backend panicking on a single file killed the whole
process, taking down a mount or every user of a serve command, even for servers
such as serve http whose library recovers panics raised on its own request
goroutines.
Recover panics at those goroutines and log them with a stack trace. Where the
surrounding code already handles a failure, recover around the backend call
itself rather than the whole goroutine, so a panicking upload is retried like
any other failed upload and a panicking download is reported to the waiters,
instead of abandoning the work part way through and leaving the bookkeeping
inconsistent.
Addresses GHSA-6jcg-q3wp-x2f4
When processItems filled the last free transfer slot it checked the
next queued item's expiry before the transfer limit, so if that
expiry was still fractionally in the future the timer was reset
instead of stopped. This caused intermittent failures in
TestWriteBackMaxQueue which asserts the timer is stopped once
--transfers uploads are in progress.
Check the transfer limit first so the timer is always stopped when
the transfer limit is reached. The timer is restarted when an upload
finishes so nothing stalls.
Also fix a typo in TestWriteBackMaxQueue which named every queued
item "number1".
Previously if a cached file had grown larger than the remote object
and the cached range metadata was out of sync with the cache file
(e.g. after an unclean shutdown) - reloading the file failed with an
"invalid seek position" error. This aborted the writeback and left the
file inaccessible.
Rclone now recovers the bytes that are still available from the remote
and logs an ERROR that the local file is likely corrupted after an
interrupted upload.
See #9231.
_createFile opened the cache file with O_RDWR but not O_CREATE, relying
on the file already existing. When the file had been removed underneath
rclone - either by _checkObject dropping a stale entry during open, or by
external deletion - the open failed and surfaced a hard "IO error" to the
application instead of recreating the file. Add O_CREATE so the cache
self-heals in that case.
When a cached file was closed, --vfs-handle-caching kept its handle and
downloaders alive for a grace period and closed them later from a timer.
The deferred close drops the item lock while tearing down the
downloaders, leaving the file handle open. A reopen landing in that
window saw no grace timer and a live handle, failed to create the cache
file with "internal error: didn't Close file" and removed the cache
file, which hung the application reopening the file.
Reopens now wait for an in-progress grace-period close to finish so they
start from a fully closed item.
See: https://forum.rclone.org/t/opening-a-recently-closed-and-cached-file-hangs-rclone/53986/
When mounting with 'remote:.' (dot notation for current directory),
the '.' is passed through toOSPath() which encodes it to full-width
'.' (U+FF0E) via EncodeDot. This caused the VFS cache data/meta root
to be written to a full-width-dot directory while the local cache
backend pointed to the canonicalized path, resulting in writes being
cached but never uploaded.
Fix by calling clean() on the relativeDirPath before toOSPath(),
so path.Clean() strips the trailing '/.' removing the dot before
encoding.
The background kicker goroutine had a bare select outside a for loop,
so the 5s ticker fired at most once before the goroutine exited. The
intent was to run every 5s for the lifetime of the Downloaders.
This wraps the select in a for loop so the ticker fires repeatedly
until ctx is cancelled.
In practice this was benign because every downloader exit and every
successful Write already calls kickWaiters, so the background kicker
is only load-bearing when a waiter is queued, no downloader is
running, and _ensureDownloader failed transiently. In that state,
before this fix, the waiter would hang until another Download() call
or Close() arrived; now it gets retried every 5s and will either
recover or accumulate enough errors to trip maxErrorCount and error
out cleanly.
Add a ctx field to the VFS struct, initialized in New() from the
existing cancellable context. Propagate this through the cache
subsystem hierarchy.
This ensures proper context cancellation when a VFS shuts down, rather
than using disconnected context.TODO() or context.Background() calls
throughout and paves the way for VFS to have its own config.
Before this change when a cache item was in its grace period (with
HandleCaching) and the file is reopened, _checkObject runs before the
grace timer recovery check. If the remote object's fingerprint changed
_checkObject removes the cache file from disk. However the grace
recovery path still reused the now-stale fd pointing to a deleted
inode, skipping _createFile entirely. This left no cache file on disk,
causing cache.Exists() to return false and breaking
rename-while-writing logic.
Fix this by checking the cache file still exists before reusing the fd
in grace recovery. If the file was removed, close the stale fd and
downloaders and fall through to _createFile.
Also update the fingerprint in item.rename after setting the new object,
preventing unnecessary cache invalidation when a file is reopened after
a rename.
This was discovered in the integration tests on backends that update
modtime on rename (like mailru).
Add a configurable grace period (default 5s) that delays closing file
handles and downloaders when the last handle closes. If a new handle
opens within the grace period, it reuses the existing resources.
This fixes 40x performance degradation with serve nfs vs serve sftp
caused by go-nfs opening/reading/closing on every NFS READ RPC, which
destroyed read-ahead prefetch before it could accumulate.
The grace period only applies to non-dirty files so that writeback
proceeds immediately on close.
Fixes#9251
This commit modernizes Go usage. This was done with:
go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -fix -test ./...
Then files needed to be `go fmt`ed and a few comments needed to be
restored.
The modernizations include replacing
- if/else conditional assignment by a call to the built-in min or max functions added in go1.21
- sort.Slice(x, func(i, j int) bool) { return s[i] < s[j] } by a call to slices.Sort(s), added in go1.21
- interface{} by the 'any' type added in go1.18
- append([]T(nil), s...) by slices.Clone(s) or slices.Concat(s), added in go1.21
- loop around an m[k]=v map update by a call to one of the Collect, Copy, Clone, or Insert functions from the maps package, added in go1.21
- []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...), added in go1.19
- append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1), added in go1.21
- a 3-clause for i := 0; i < n; i++ {} loop by for i := range n {}, added in go1.22
Before this change, if --vfs-cache-mode writes or above was set and
--links was in use, when a symlink was saved then the VFS failed to
upload it. This meant when the VFS was restarted the link wasn't there
any more.
This was caused by the local backend, which we use to manage the VFS
cache, picking up the global --links flag.
This patch makes sure that the internal instantations of the local
backend in the VFS cache don't ever use the --links flag or the
--local-links flag even if specified on the command line.
Fixes#8367
This converts the ChunkedReader into an interface and provides two
implementations one sequential and one parallel.
This can be used to improve the performance of the VFS on high
bandwidth or high latency links.
Fixes#4760
The vfs use the hardcoded OS encoding when creating temp file,
but decode it with encoding for the local filesystem (--local-encoding)
when copying it to remote.
This caused failures when the filenames contained special characters.
The hardcoded OS encoding is now used uniformly.
This also
- move in use options (Opt) from vfsflags to vfscommon
- change os.FileMode to vfscommon.FileMode in parameters
- rework vfscommon.FileMode and add tests
Before this change a panic could be overwritten with the message
fatal error: sync: unlock of unlocked mutex
This was because we temporarily unlocked the mutex, but failed to lock
it again if there was a panic.
This is code is never the cause of an error but it masks the
underlying error by overwriting the panic cause.
See: https://forum.rclone.org/t/serve-webdav-is-crashing-fatal-error-sync-unlock-of-unlocked-mutex/46300
Before this change, if a file shrunk in size on the remote then rclone
could get into an loop trying to download the file forever.
The symptom was repeating errors like this:
vfs cache: restart download failed: failed to start downloader: failed to open downloader: vfs reader: failed to open source file: invalid seek position
The fix was to check that file size in various places and makes sure
that we weren't trying to download too much data.
This was a problems with backends (like s3) which update the size of
the object on Open to the actual size of the object.
Before this change it wasn't possible to see where transfers were
going from and to in core/stats and core/transferred.
When use in rclone mount in particular this made interpreting the
stats very hard.
Before this change the VFS cache could get into a state where when an
object was updated remotely, the fingerprint of the item was correct
for the new object but the data in the VFS cache was for the old
object.
This fixes the problem by updating the fingerprint of the item at the
point we remove the stale data. The empty cache item now represents
the new item even though it has no data in.
This stops the fallback code for an empty fingerprint running (used
when we are writing items to the cache instead of reading them) which
was causing the problem.
Fixes#6053
See: https://forum.rclone.org/t/cached-webdav-mount-fingerprints-get-nuked-on-ls/43974/
Before this change if a backend can't upload 0 length files and
`--vfs-cache-mode writes` was in use then the writeback logic would
try to upload the 0 length file forever.
This change causes it to exit on the first failure to upload.
Before this patch a deadlock could occur if the cache cleaner was
running when an object upload finished.
This fixes the problem by delaying marking the object as clean until
we have notified the VFS layer. This means that the cache cleaner
won't consider the object until **after** the VFS layer has been
notified, thus avoiding the deadlock.
See: https://forum.rclone.org/t/rclone-mount-deadlock-when-dir-cache-time-strikes/33486/
Before this change we truncated files in the backing store regardless
of whether we needed to or not.
After, we check to see if the file is the right size and don't
truncate if it is.
Apparently Windows Defender likes to check executables each time they
are modified, and truncating a file to its existing size is enough to
trigger the Windows Defender scan. This was causing a big slowdown for
operations which opened and closed the file a lot, such as looking at
properties on an executable.
See: https://forum.rclone.org/t/for-mount-sftp-why-right-click-on-exe-file-is-so-slow-until-it-freezes/33830
Before this change if we copied files of unknown size, then they lost
their metadata.
This was particularly noticeable using --s3-decompress.
This change adds metadata to Rcat and RcatSized and changes Copy to
pass the metadata in when it calls Rcat for an unknown sized input.
Fixes#6546
Before this change the VFS cache cleaner would loop indefinitely while
the cache was above quota. This used up all the CPU.
This fix prevents the cache cleaner from looping. It will be kicked on
ENOSPACE and run in its scheduled time otherwise so this should be
sufficient.
See: https://forum.rclone.org/t/vfs-keeps-checking-same-files/32120
ferrumclaudepilgrim
Vijay Misal
Nick Craig-Wood
Dave
Lucky945H
yy
Dimitri Papadopoulos
URenko
Anagh Kumar Baranwal
Roberto Ricci
albertony
Eng Zer Jun
Josh Soref