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
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user