From 6e0c71bd276bd587403fd00e88ef195aa6996789 Mon Sep 17 00:00:00 2001 From: Dave Date: Fri, 14 Aug 2026 08:14:01 +0200 Subject: [PATCH] vfs/vfscache: fix reader deadlock when the item size drops below the read offset _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 --- vfs/vfscache/downloaders/downloaders.go | 6 +- vfs/vfscache/downloaders/downloaders_test.go | 25 +++++++ vfs/vfscache/item_waiter_test.go | 72 ++++++++++++++++++++ 3 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 vfs/vfscache/item_waiter_test.go diff --git a/vfs/vfscache/downloaders/downloaders.go b/vfs/vfscache/downloaders/downloaders.go index 333edc8fc..39cd4e883 100644 --- a/vfs/vfscache/downloaders/downloaders.go +++ b/vfs/vfscache/downloaders/downloaders.go @@ -394,7 +394,11 @@ func (dls *Downloaders) _dispatchWaiters() { // Clip the size against the actual size in case it has shrunk r := waiter.r r.Clip(dls.src.Size()) - if dls.item.HasRange(r) { + // Wake the waiter if its data has arrived, or if there is nothing + // left to download for it. _ensureDownloader starts a downloader + // only when FindMissing is non empty, so a waiter with nothing + // missing would otherwise never be woken by anything. + if dls.item.HasRange(r) || dls.item.FindMissing(waiter.r).IsEmpty() { waiter.errChan <- nil } else { newWaiters = append(newWaiters, waiter) diff --git a/vfs/vfscache/downloaders/downloaders_test.go b/vfs/vfscache/downloaders/downloaders_test.go index b38abb0eb..592c3f470 100644 --- a/vfs/vfscache/downloaders/downloaders_test.go +++ b/vfs/vfscache/downloaders/downloaders_test.go @@ -126,4 +126,29 @@ func TestDownloaders(t *testing.T) { return item.HasRange(r) }, 10*time.Second, 10*time.Millisecond) }) + + // A waiter for a range beyond the item's size but within the source + // object's size must still be dispatched. _ensureDownloader will not + // start a downloader for it, so nothing else would ever wake it. + t.Run("DownloadBeyondItemSize", func(t *testing.T) { + item := &testItem{ + t: t, + size: 1024 * 1024, + } + opt := vfscommon.Opt + dls := New(ctx, item, &opt, remote, src) + defer cancel(dls) + + done := make(chan error, 1) + go func() { + done <- dls.Download(ranges.Range{Pos: 40 * 1024 * 1024, Size: 250}) + }() + + select { + case err := <-done: + assert.NoError(t, err) + case <-time.After(30 * time.Second): + t.Fatal("Download did not return: the waiter was never dispatched") + } + }) } diff --git a/vfs/vfscache/item_waiter_test.go b/vfs/vfscache/item_waiter_test.go new file mode 100644 index 000000000..49a921d43 --- /dev/null +++ b/vfs/vfscache/item_waiter_test.go @@ -0,0 +1,72 @@ +package vfscache + +import ( + "os" + "testing" + "time" + + "github.com/rclone/rclone/lib/ranges" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestItemWaiterBeyondItemSize checks that a reader waiting for a range past +// the item's size, but within the size of the source object the downloaders +// hold, is still released. +// +// Download is called directly because _ensure clips the range against +// info.Size before it can reach the waiter queue. In the field the waiter is +// queued while info.Size is still full and info.Size shrinks afterwards. +func TestItemWaiterBeyondItemSize(t *testing.T) { + r, c := newItemTestCache(t) + + const ( + remote = "waiter.bin" + fileSize = 64 * 1024 * 1024 + headLen = 64 * 1024 + shrunkSize = 1024 * 1024 + tailOffset = 48 * 1024 * 1024 + ) + + _, obj, item := newFileLength(t, r, c, remote, fileSize) + require.NoError(t, item.Open(obj)) + defer func() { _ = item.Close(nil) }() + + buf := make([]byte, headLen) + n, err := item.ReadAt(buf, 0) + require.NoError(t, err) + require.Equal(t, headLen, n) + + osPath := c.toOSPath(remote) + apparent := func() int64 { + fi, statErr := os.Stat(osPath) + require.NoError(t, statErr) + return fi.Size() + } + require.Equal(t, int64(fileSize), obj.Size()) + require.Equal(t, int64(fileSize), apparent()) + + head := ranges.Range{Pos: 0, Size: headLen} + tail := ranges.Range{Pos: tailOffset, Size: 4096} + require.True(t, item.info.Rs.Present(head)) + require.False(t, item.info.Rs.Present(tail), "tail must not be prefetched") + + // Shrink info.Size below the tail offset, as readAt does when the + // remote object and the item disagree about the size. + item.mu.Lock() + require.NoError(t, item._truncate(shrunkSize)) + item.mu.Unlock() + + require.Equal(t, int64(shrunkSize), item.info.Size) + require.Equal(t, int64(shrunkSize), apparent()) + + done := make(chan error, 1) + go func() { done <- item.downloaders.Download(tail) }() + + select { + case err := <-done: + assert.NoError(t, err) + case <-time.After(30 * time.Second): + t.Fatal("Download did not return: the waiter was never dispatched") + } +}