nielash
71a0932126
Before this change, Transfer.Done closed the account of a completed transfer but never released it, because it assigned nil to only a local copy of the pointer. As a result, every completed transfer continued to reference its account. An Account holds the transfer context and the source reader, and the stats keep completed transfers around up to `MaxCompletedTransfers` per group, with groups discarded only at --max-stats-groups. On a long-running `rclone rcd`, this adds up. It was noticeable when running bisync repeatedly via the rc, where the transfer context carries `b.WriteResults` (bisync's `LoggerFn`). A `*bisyncRun` holds Path1 and Path2 listings, which can be quite large, and are not supposed to be retained between runs. (Naturally they aren't, when running bisync on the command line -- which is probably why we didn't notice this issue sooner.) This change fixes the issue by releasing `tr.acc` in Done, instead of the local copy. `tr.Snapshot` reads the byte counts off the account, so the progress is recorded on the transfer first. That read happens before taking `tr.mu`, because `acc.progress()` locks `acc.values.mu`, `checkReadBefore` holds that lock while calling `StatsInfo.GetBytes`, and `StatsInfo` locks back into `Transfer` in `Transferred` and `_removeTransfer`.
107 lines
2.9 KiB
Go
107 lines
2.9 KiB
Go
package accounting
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"io"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/rclone/rclone/fs/rc"
|
|
"github.com/rclone/rclone/fstest/mockfs"
|
|
"github.com/rclone/rclone/fstest/mockobject"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
func TestTransfer(t *testing.T) {
|
|
ctx := context.Background()
|
|
s := NewStats(ctx)
|
|
|
|
o := mockobject.Object("obj")
|
|
srcFs, err := mockfs.NewFs(ctx, "srcFs", "srcFs", nil)
|
|
require.NoError(t, err)
|
|
dstFs, err := mockfs.NewFs(ctx, "dstFs", "dstFs", nil)
|
|
require.NoError(t, err)
|
|
|
|
tr := newTransfer(s, o, srcFs, dstFs)
|
|
|
|
t.Run("Snapshot", func(t *testing.T) {
|
|
snap := tr.Snapshot()
|
|
assert.Equal(t, "obj", snap.Name)
|
|
assert.Equal(t, int64(0), snap.Size)
|
|
assert.Equal(t, int64(0), snap.Bytes)
|
|
assert.Equal(t, false, snap.Checked)
|
|
assert.Equal(t, "transferring", snap.What)
|
|
assert.Equal(t, false, snap.StartedAt.IsZero())
|
|
assert.Equal(t, true, snap.CompletedAt.IsZero())
|
|
assert.Equal(t, nil, snap.Error)
|
|
assert.Equal(t, "", snap.Group)
|
|
assert.Equal(t, "srcFs:srcFs", snap.SrcFs)
|
|
assert.Equal(t, "dstFs:dstFs", snap.DstFs)
|
|
})
|
|
|
|
t.Run("Done", func(t *testing.T) {
|
|
tr.Done(ctx, io.EOF)
|
|
snap := tr.Snapshot()
|
|
assert.Equal(t, "obj", snap.Name)
|
|
assert.Equal(t, int64(0), snap.Size)
|
|
assert.Equal(t, int64(0), snap.Bytes)
|
|
assert.Equal(t, false, snap.Checked)
|
|
assert.Equal(t, "transferring", snap.What)
|
|
assert.Equal(t, false, snap.StartedAt.IsZero())
|
|
assert.Equal(t, false, snap.CompletedAt.IsZero())
|
|
assert.Equal(t, true, errors.Is(snap.Error, io.EOF))
|
|
assert.Equal(t, "", snap.Group)
|
|
assert.Equal(t, "srcFs:srcFs", snap.SrcFs)
|
|
assert.Equal(t, "dstFs:dstFs", snap.DstFs)
|
|
})
|
|
|
|
t.Run("DoneReleasesAccount", func(t *testing.T) {
|
|
content := "hello world"
|
|
o := mockobject.New("obj").WithContent([]byte(content), mockobject.SeekModeNone)
|
|
tr := newTransfer(s, o, srcFs, dstFs)
|
|
in := tr.Account(ctx, io.NopCloser(strings.NewReader(content)))
|
|
_, err := io.Copy(io.Discard, in)
|
|
require.NoError(t, err)
|
|
|
|
tr.Done(ctx, nil)
|
|
|
|
tr.mu.RLock()
|
|
acc := tr.acc
|
|
tr.mu.RUnlock()
|
|
assert.Nil(t, acc)
|
|
|
|
snap := tr.Snapshot()
|
|
assert.Equal(t, int64(len(content)), snap.Bytes)
|
|
assert.Equal(t, int64(len(content)), snap.Size)
|
|
})
|
|
|
|
t.Run("rcStats", func(t *testing.T) {
|
|
out := tr.rcStats()
|
|
assert.Equal(t, rc.Params{
|
|
"name": "obj",
|
|
"size": int64(0),
|
|
"srcFs": "srcFs:srcFs",
|
|
"dstFs": "dstFs:dstFs",
|
|
}, out)
|
|
})
|
|
|
|
t.Run("Snapshot checking transfer", func(t *testing.T) {
|
|
ctr := newCheckingTransfer(s, o, "checking")
|
|
snap := ctr.Snapshot()
|
|
|
|
assert.Equal(t, "obj", snap.Name)
|
|
assert.Equal(t, int64(0), snap.Size)
|
|
assert.Equal(t, int64(0), snap.Bytes)
|
|
assert.Equal(t, true, snap.Checked)
|
|
assert.Equal(t, "checking", snap.What)
|
|
assert.Equal(t, false, snap.StartedAt.IsZero())
|
|
assert.Equal(t, true, snap.CompletedAt.IsZero())
|
|
assert.Equal(t, nil, snap.Error)
|
|
assert.Equal(t, "", snap.Group)
|
|
assert.Equal(t, "", snap.SrcFs)
|
|
assert.Equal(t, "", snap.DstFs)
|
|
})
|
|
}
|