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") + } +}