diff --git a/lib/model/folder_sendrecv_test.go b/lib/model/folder_sendrecv_test.go index 176e6cd2d..b6dfb6f56 100644 --- a/lib/model/folder_sendrecv_test.go +++ b/lib/model/folder_sendrecv_test.go @@ -19,8 +19,6 @@ import ( "time" "github.com/syncthing/syncthing/lib/config" - "github.com/syncthing/syncthing/lib/db" - "github.com/syncthing/syncthing/lib/db/backend" "github.com/syncthing/syncthing/lib/events" "github.com/syncthing/syncthing/lib/fs" "github.com/syncthing/syncthing/lib/ignore" @@ -90,37 +88,20 @@ func createFile(t *testing.T, name string, fs fs.Filesystem) protocol.FileInfo { return file } +// Sets up a folder and model, but makes sure the services aren't actually running. func setupSendReceiveFolder(files ...protocol.FileInfo) (*model, *sendReceiveFolder) { - w := createTmpWrapper(defaultCfg) - model := newModel(w, myID, "syncthing", "dev", db.NewLowlevel(backend.OpenMemory()), nil) - fcfg := testFolderConfigTmp() - model.addFolder(fcfg) - - f := &sendReceiveFolder{ - folder: folder{ - stateTracker: newStateTracker("default", model.evLogger), - model: model, - fset: model.folderFiles[fcfg.ID], - initialScanFinished: make(chan struct{}), - ctx: context.TODO(), - FolderConfiguration: fcfg, - }, - - queue: newJobQueue(), - writeLimiter: newByteSemaphore(2), - pullErrors: make(map[string]string), - pullErrorsMut: sync.NewMutex(), - } - f.fs = fs.NewMtimeFS(f.Filesystem(), db.NewNamespacedKV(model.db, "mtime")) + w, fcfg := tmpDefaultWrapper() + model := setupModel(w) + model.Supervisor.Stop() + f := model.folderRunners[fcfg.ID].(*sendReceiveFolder) + f.pullErrors = make(map[string]string) + f.ctx = context.Background() // Update index if files != nil { f.updateLocalsFromScanning(files) } - // Folders are never actually started, so no initial scan will be done - close(f.initialScanFinished) - return model, f } @@ -392,7 +373,7 @@ func TestWeakHash(t *testing.T) { case pull := <-pullChan: pulls = append(pulls, pull) case <-timeout: - t.Errorf("timed out, got %d pulls expected %d", len(pulls), expectPulls) + t.Fatalf("timed out, got %d pulls expected %d", len(pulls), expectPulls) } } finish := <-finisherChan @@ -420,7 +401,7 @@ func TestWeakHash(t *testing.T) { case pull := <-pullChan: pulls = append(pulls, pull) case <-time.After(10 * time.Second): - t.Errorf("timed out, got %d pulls expected %d", len(pulls), expectPulls) + t.Fatalf("timed out, got %d pulls expected %d", len(pulls), expectPulls) } } @@ -489,7 +470,7 @@ func TestDeregisterOnFailInCopy(t *testing.T) { } pullChan := make(chan pullBlockState) - finisherBufferChan := make(chan *sharedPullerState) + finisherBufferChan := make(chan *sharedPullerState, 1) finisherChan := make(chan *sharedPullerState) dbUpdateChan := make(chan dbUpdateJob, 1) snap := f.fset.Snapshot() @@ -509,7 +490,12 @@ func TestDeregisterOnFailInCopy(t *testing.T) { // Receive a block at puller, to indicate that at least a single copier // loop has been performed. - toPull := <-pullChan + var toPull pullBlockState + select { + case toPull = <-pullChan: + case <-time.After(10 * time.Second): + t.Fatal("timed out") + } // Unblock copier go func() { diff --git a/lib/model/model.go b/lib/model/model.go index 56b265fd6..25d4793fd 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -275,22 +275,22 @@ func (m *model) StartDeadlockDetector(timeout time.Duration) { detector.Watch("pmut", m.pmut) } -// startFolder constructs the folder service and starts it. -func (m *model) startFolder(folder string) { - m.fmut.RLock() - folderCfg := m.folderCfgs[folder] - m.fmut.RUnlock() +// Need to hold lock on m.fmut when calling this. +func (m *model) addAndStartFolderLocked(cfg config.FolderConfiguration, fset *db.FileSet) { + ignores := ignore.New(cfg.Filesystem(), ignore.WithCache(m.cacheIgnoredFiles)) + if err := ignores.Load(".stignore"); err != nil && !fs.IsNotExist(err) { + l.Warnln("Loading ignores:", err) + } - // Close connections to affected devices - m.closeConns(folderCfg.DeviceIDs(), fmt.Errorf("started folder %v", folderCfg.Description())) - - m.fmut.Lock() - defer m.fmut.Unlock() - m.startFolderLocked(folderCfg) + m.addAndStartFolderLockedWithIgnores(cfg, fset, ignores) } -// Need to hold lock on m.fmut when calling this. -func (m *model) startFolderLocked(cfg config.FolderConfiguration) { +// Only needed for testing, use addAndStartFolderLocked instead. +func (m *model) addAndStartFolderLockedWithIgnores(cfg config.FolderConfiguration, fset *db.FileSet, ignores *ignore.Matcher) { + m.folderCfgs[cfg.ID] = cfg + m.folderFiles[cfg.ID] = fset + m.folderIgnores[cfg.ID] = ignores + _, ok := m.folderRunners[cfg.ID] if ok { l.Warnln("Cannot start already running folder", cfg.Description()) @@ -304,8 +304,6 @@ func (m *model) startFolderLocked(cfg config.FolderConfiguration) { folder := cfg.ID - fset := m.folderFiles[folder] - // Find any devices for which we hold the index in the db, but the folder // is not shared, and drop it. expected := mapDevices(cfg.DeviceIDs()) @@ -356,8 +354,6 @@ func (m *model) startFolderLocked(cfg config.FolderConfiguration) { } m.folderVersioners[folder] = ver - ignores := m.folderIgnores[folder] - p := folderFactory(m, fset, ignores, cfg, ver, ffs, m.evLogger, m.folderIOLimiter) m.folderRunners[folder] = p @@ -403,35 +399,6 @@ func (m *model) warnAboutOverwritingProtectedFiles(cfg config.FolderConfiguratio } } -func (m *model) addFolder(cfg config.FolderConfiguration) { - if len(cfg.ID) == 0 { - panic("cannot add empty folder id") - } - - if len(cfg.Path) == 0 { - panic("cannot add empty folder path") - } - - // Creating the fileset can take a long time (metadata calculation) so - // we do it outside of the lock. - fset := db.NewFileSet(cfg.ID, cfg.Filesystem(), m.db) - - m.fmut.Lock() - defer m.fmut.Unlock() - m.addFolderLocked(cfg, fset) -} - -func (m *model) addFolderLocked(cfg config.FolderConfiguration, fset *db.FileSet) { - m.folderCfgs[cfg.ID] = cfg - m.folderFiles[cfg.ID] = fset - - ignores := ignore.New(cfg.Filesystem(), ignore.WithCache(m.cacheIgnoredFiles)) - if err := ignores.Load(".stignore"); err != nil && !fs.IsNotExist(err) { - l.Warnln("Loading ignores:", err) - } - m.folderIgnores[cfg.ID] = ignores -} - func (m *model) removeFolder(cfg config.FolderConfiguration) { m.stopFolder(cfg, fmt.Errorf("removing folder %v", cfg.Description())) @@ -449,7 +416,7 @@ func (m *model) removeFolder(cfg config.FolderConfiguration) { cfg.Filesystem().RemoveAll(config.DefaultMarkerName) } - m.removeFolderLocked(cfg) + m.cleanupFolderLocked(cfg) m.fmut.Unlock() @@ -474,7 +441,7 @@ func (m *model) stopFolder(cfg config.FolderConfiguration, err error) { } // Need to hold lock on m.fmut when calling this. -func (m *model) removeFolderLocked(cfg config.FolderConfiguration) { +func (m *model) cleanupFolderLocked(cfg config.FolderConfiguration) { // Clean up our config maps delete(m.folderCfgs, cfg.ID) delete(m.folderFiles, cfg.ID) @@ -529,10 +496,9 @@ func (m *model) restartFolder(from, to config.FolderConfiguration) { m.fmut.Lock() defer m.fmut.Unlock() - m.removeFolderLocked(from) + m.cleanupFolderLocked(from) if !to.Paused { - m.addFolderLocked(to, fset) - m.startFolderLocked(to) + m.addAndStartFolderLocked(to, fset) } l.Infof("%v folder %v (%v)", infoMsg, to.Description(), to.Type) } @@ -547,8 +513,7 @@ func (m *model) newFolder(cfg config.FolderConfiguration) { m.fmut.Lock() defer m.fmut.Unlock() - m.addFolderLocked(cfg, fset) - m.startFolderLocked(cfg) + m.addAndStartFolderLocked(cfg, fset) } func (m *model) UsageReportingStats(version int, preview bool) map[string]interface{} { diff --git a/lib/model/model_test.go b/lib/model/model_test.go index 4cb543d99..73f9a0728 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -410,10 +410,6 @@ func TestClusterConfig(t *testing.T) { wrapper := createTmpWrapper(cfg) m := newModel(wrapper, myID, "syncthing", "dev", db, nil) m.ServeBackground() - for _, fcfg := range cfg.Folders { - m.removeFolder(fcfg) - m.addFolder(fcfg) - } defer cleanupModel(m) cm := m.generateClusterConfig(device2) @@ -1460,16 +1456,7 @@ func TestIgnores(t *testing.T) { m := setupModel(defaultCfgWrapper) defer cleanupModel(m) - m.removeFolder(defaultFolderConfig) - m.addFolder(defaultFolderConfig) - // Reach in and update the ignore matcher to one that always does - // reloads when asked to, instead of checking file mtimes. This is - // because we will be changing the files on disk often enough that the - // mtimes will be unreliable to determine change status. - m.fmut.Lock() - m.folderIgnores["default"] = ignore.New(defaultFs, ignore.WithCache(true), ignore.WithChangeDetector(newAlwaysChanged())) - m.fmut.Unlock() - m.startFolder("default") + folderIgnoresAlwaysReload(m, defaultFolderConfig) // Make sure the initial scan has finished (ScanFolders is blocking) m.ScanFolders() @@ -1492,7 +1479,13 @@ func TestIgnores(t *testing.T) { } // Invalid path, marker should be missing, hence returns an error. - m.addFolder(config.FolderConfiguration{ID: "fresh", Path: "XXX"}) + fcfg := config.FolderConfiguration{ID: "fresh", Path: "XXX"} + ignores := ignore.New(fcfg.Filesystem(), ignore.WithCache(m.cacheIgnoredFiles)) + m.fmut.Lock() + m.folderCfgs[fcfg.ID] = fcfg + m.folderIgnores[fcfg.ID] = ignores + m.fmut.Unlock() + _, _, err = m.GetIgnores("fresh") if err == nil { t.Error("No error") @@ -1526,9 +1519,6 @@ func TestEmptyIgnores(t *testing.T) { m := setupModel(defaultCfgWrapper) defer cleanupModel(m) - m.removeFolder(defaultFolderConfig) - m.addFolder(defaultFolderConfig) - if err := m.SetIgnores("default", []string{}); err != nil { t.Error(err) } @@ -1685,8 +1675,6 @@ func TestGlobalDirectoryTree(t *testing.T) { db := db.NewLowlevel(backend.OpenMemory()) m := newModel(defaultCfgWrapper, myID, "syncthing", "dev", db, nil) m.ServeBackground() - m.removeFolder(defaultFolderConfig) - m.addFolder(defaultFolderConfig) defer cleanupModel(m) b := func(isfile bool, path ...string) protocol.FileInfo { @@ -1938,8 +1926,6 @@ func TestGlobalDirectorySelfFixing(t *testing.T) { db := db.NewLowlevel(backend.OpenMemory()) m := newModel(defaultCfgWrapper, myID, "syncthing", "dev", db, nil) m.ServeBackground() - m.removeFolder(defaultFolderConfig) - m.addFolder(defaultFolderConfig) defer cleanupModel(m) b := func(isfile bool, path ...string) protocol.FileInfo { @@ -2115,8 +2101,6 @@ func benchmarkTree(b *testing.B, n1, n2 int) { db := db.NewLowlevel(backend.OpenMemory()) m := newModel(defaultCfgWrapper, myID, "syncthing", "dev", db, nil) m.ServeBackground() - m.removeFolder(defaultFolderConfig) - m.addFolder(defaultFolderConfig) defer cleanupModel(m) m.ScanFolder("default") @@ -2313,8 +2297,7 @@ func TestIndexesForUnknownDevicesDropped(t *testing.T) { } m := newModel(defaultCfgWrapper, myID, "syncthing", "dev", dbi, nil) - m.addFolder(defaultFolderConfig) - m.startFolder("default") + m.newFolder(defaultFolderConfig) defer cleanupModel(m) // Remote sequence is cached, hence need to recreated. diff --git a/lib/model/requests_test.go b/lib/model/requests_test.go index 3df942824..7932353ce 100644 --- a/lib/model/requests_test.go +++ b/lib/model/requests_test.go @@ -22,7 +22,6 @@ import ( "github.com/syncthing/syncthing/lib/config" "github.com/syncthing/syncthing/lib/events" "github.com/syncthing/syncthing/lib/fs" - "github.com/syncthing/syncthing/lib/ignore" "github.com/syncthing/syncthing/lib/protocol" ) @@ -302,16 +301,7 @@ func pullInvalidIgnored(t *testing.T, ft config.FolderType) { m := setupModel(w) defer cleanupModelAndRemoveDir(m, fss.URI()) - m.removeFolder(fcfg) - m.addFolder(fcfg) - // Reach in and update the ignore matcher to one that always does - // reloads when asked to, instead of checking file mtimes. This is - // because we might be changing the files on disk often enough that the - // mtimes will be unreliable to determine change status. - m.fmut.Lock() - m.folderIgnores["default"] = ignore.New(fss, ignore.WithChangeDetector(newAlwaysChanged())) - m.fmut.Unlock() - m.startFolder(fcfg.ID) + folderIgnoresAlwaysReload(m, fcfg) fc := addFakeConn(m, device1) fc.folder = "default" @@ -1039,16 +1029,7 @@ func TestIgnoreDeleteUnignore(t *testing.T) { tmpDir := fss.URI() defer cleanupModelAndRemoveDir(m, tmpDir) - m.removeFolder(fcfg) - m.addFolder(fcfg) - // Reach in and update the ignore matcher to one that always does - // reloads when asked to, instead of checking file mtimes. This is - // because we might be changing the files on disk often enough that the - // mtimes will be unreliable to determine change status. - m.fmut.Lock() - m.folderIgnores["default"] = ignore.New(fss, ignore.WithChangeDetector(newAlwaysChanged())) - m.fmut.Unlock() - m.startFolder(fcfg.ID) + folderIgnoresAlwaysReload(m, fcfg) fc := addFakeConn(m, device1) fc.folder = "default" diff --git a/lib/model/testutils_test.go b/lib/model/testutils_test.go index eada47020..dd0d6c6bb 100644 --- a/lib/model/testutils_test.go +++ b/lib/model/testutils_test.go @@ -17,6 +17,7 @@ import ( "github.com/syncthing/syncthing/lib/db/backend" "github.com/syncthing/syncthing/lib/events" "github.com/syncthing/syncthing/lib/fs" + "github.com/syncthing/syncthing/lib/ignore" "github.com/syncthing/syncthing/lib/protocol" "github.com/syncthing/syncthing/lib/rand" ) @@ -217,3 +218,16 @@ func dbSnapshot(t *testing.T, m Model, folder string) *db.Snapshot { } return snap } + +// Reach in and update the ignore matcher to one that always does +// reloads when asked to, instead of checking file mtimes. This is +// because we will be changing the files on disk often enough that the +// mtimes will be unreliable to determine change status. +func folderIgnoresAlwaysReload(m *model, fcfg config.FolderConfiguration) { + m.removeFolder(fcfg) + fset := db.NewFileSet(fcfg.ID, fcfg.Filesystem(), m.db) + ignores := ignore.New(fcfg.Filesystem(), ignore.WithCache(true), ignore.WithChangeDetector(newAlwaysChanged())) + m.fmut.Lock() + m.addAndStartFolderLockedWithIgnores(fcfg, fset, ignores) + m.fmut.Unlock() +}