Package: syncthing Version: 1.12.1~ds1-2 Severity: important As discussed on the go-team mailing list [1], I am filing this bug to address an important fix in upstream since the version to be included in stable.
There's a bug in the code that determines sync status when removing a device (or resetting an index). This may leed to incorrect remote sync status. The patch fixing this is attached. [1]: https://lists.debian.org/debian-go/2021/02/msg00067.html
>From 631cf607495f4a21c72c731f3fb54bbeacd9af15 Mon Sep 17 00:00:00 2001 From: Simon Frei <[email protected]> Date: Mon, 8 Feb 2021 08:38:41 +0100 Subject: [PATCH 4/4] lib/db: Fix and improve removing entries from global (ref #6501) (#7336) --- lib/db/db_test.go | 43 ++++++++++++++++++++++++++++++++++++++++++ lib/db/transactions.go | 27 ++++++++++++++++---------- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/lib/db/db_test.go b/lib/db/db_test.go index 07c2ae7bc..886e955fd 100644 --- a/lib/db/db_test.go +++ b/lib/db/db_test.go @@ -942,6 +942,49 @@ func TestDuplicateNeedCount(t *testing.T) { } } +func TestNeedAfterDropGlobal(t *testing.T) { + db := NewLowlevel(backend.OpenMemory()) + defer db.Close() + + folder := "test" + testFs := fs.NewFilesystem(fs.FilesystemTypeFake, "") + + fs := NewFileSet(folder, testFs, db) + + // Initial: + // Three devices and a file "test": local has Version 1, remoteDevice0 + // Version 2 and remoteDevice2 doesn't have it. + // All of them have "bar", just so the db knows about remoteDevice2. + files := []protocol.FileInfo{ + {Name: "foo", Version: protocol.Vector{}.Update(myID), Sequence: 1}, + {Name: "bar", Version: protocol.Vector{}.Update(myID), Sequence: 2}, + } + fs.Update(protocol.LocalDeviceID, files) + files[0].Version = files[0].Version.Update(myID) + fs.Update(remoteDevice0, files) + fs.Update(remoteDevice1, files[1:]) + + // remoteDevice1 needs one file: test + snap := fs.Snapshot() + c := snap.NeedSize(remoteDevice1) + if c.Files != 1 { + t.Errorf("Expected 1 needed files initially, got %v", c.Files) + } + snap.Release() + + // Drop remoteDevice0, i.e. remove all their files from db. + // That changes the global file, which is now what local has. + fs.Drop(remoteDevice0) + + // remoteDevice1 still needs test. + snap = fs.Snapshot() + c = snap.NeedSize(remoteDevice1) + if c.Files != 1 { + t.Errorf("Expected still 1 needed files, got %v", c.Files) + } + snap.Release() +} + func numBlockLists(db *Lowlevel) (int, error) { it, err := db.Backend.NewPrefixIterator([]byte{KeyTypeBlockList}) if err != nil { diff --git a/lib/db/transactions.go b/lib/db/transactions.go index 02c9a8e05..e97dcdd25 100644 --- a/lib/db/transactions.go +++ b/lib/db/transactions.go @@ -813,11 +813,11 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device, file } var global protocol.FileIntf - var gotGlobal, ok bool + var gotGlobal bool - globalFV, ok := fl.GetGlobal() + globalFV, haveGlobal := fl.GetGlobal() // Add potential needs of the removed device - if ok && !globalFV.IsInvalid() && Need(globalFV, false, protocol.Vector{}) && !Need(oldGlobalFV, haveRemoved, removedFV.Version) { + if haveGlobal && !globalFV.IsInvalid() && Need(globalFV, false, protocol.Vector{}) && !Need(oldGlobalFV, haveRemoved, removedFV.Version) { keyBuf, global, _, err = t.getGlobalFromVersionList(keyBuf, folder, file, true, fl) if err != nil { return nil, err @@ -840,16 +840,23 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device, file return keyBuf, nil } - var f protocol.FileIntf - keyBuf, f, err = t.getGlobalFromFileVersion(keyBuf, folder, file, true, oldGlobalFV) + var oldGlobal protocol.FileIntf + keyBuf, oldGlobal, err = t.getGlobalFromFileVersion(keyBuf, folder, file, true, oldGlobalFV) if err != nil { return nil, err } - meta.removeFile(protocol.GlobalDeviceID, f) + meta.removeFile(protocol.GlobalDeviceID, oldGlobal) // Remove potential device needs - if fv, have := fl.Get(protocol.LocalDeviceID[:]); Need(removedFV, have, fv.Version) { - meta.removeNeeded(protocol.LocalDeviceID, f) + shouldRemoveNeed := func(dev protocol.DeviceID) bool { + fv, have := fl.Get(dev[:]) + if !Need(oldGlobalFV, have, fv.Version) { + return false // Didn't need it before + } + return !haveGlobal || !Need(globalFV, have, fv.Version) + } + if shouldRemoveNeed(protocol.LocalDeviceID) { + meta.removeNeeded(protocol.LocalDeviceID, oldGlobal) if keyBuf, err = t.updateLocalNeed(keyBuf, folder, file, false); err != nil { return nil, err } @@ -858,8 +865,8 @@ func (t readWriteTransaction) removeFromGlobal(gk, keyBuf, folder, device, file if bytes.Equal(dev[:], device) { // Was the previous global continue } - if fv, have := fl.Get(dev[:]); Need(removedFV, have, fv.Version) { - meta.removeNeeded(dev, f) + if shouldRemoveNeed(dev) { + meta.removeNeeded(dev, oldGlobal) } } -- 2.30.0
