diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 306ce6ff..cf622cd1 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -236,6 +236,17 @@ func New(opts Options) (*Daemon, error) { // Watcher-detected save change: push it to online peers. Bound to // the P2P engine's lifecycle so shutdown cancels a transfer in // flight instead of leaving it writing into the save folder. + // + // Not for a game whose auto-sync is off: a change reported after + // it was switched off — by the catch-up, or a watch that finished + // its snapshot as it was being stopped — is the game's, but syncing + // it is what the setting says not to do. + if game, err := d.Store.GetGame(gameID); err != nil || !game.AutoSync { + if d.OnGameChanged != nil { + d.OnGameChanged(gameID) + } + return + } d.P2P.GoSync(func(ctx context.Context) { ctx, cancel := context.WithTimeout(ctx, 10*time.Minute) defer cancel() diff --git a/internal/watcher/stopped_watch_test.go b/internal/watcher/stopped_watch_test.go new file mode 100644 index 00000000..992b170b --- /dev/null +++ b/internal/watcher/stopped_watch_test.go @@ -0,0 +1,65 @@ +package watcher + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +// A watch stopped while its snapshot is being taken keeps the snapshot and +// announces nothing. +// +// A snapshot can take longer than stopping waits for the watch, so it finished +// after the watch had been given up, and announced the change: the daemon then +// synced a game whose auto-sync had just been switched off. On Windows CI that +// was the sync nobody expected in +// TestMultiRootScenario_ALateReportDoesNotHideADivergence ("the watcher ... +// did not stop within 5s and was abandoned", then "auto-snapshot created" and +// a sync). +func TestAWatchStoppedDuringItsSnapshotAnnouncesNothing(t *testing.T) { + saveDir := t.TempDir() + col := newCollector() + cb := col.callbacks() + inSnapshot := make(chan struct{}, 1) + release := make(chan struct{}) + take := cb.CreateSnapshot + cb.CreateSnapshot = func(gameID string) error { + select { + case inSnapshot <- struct{}{}: + default: + } + <-release + return take(gameID) + } + eng := New(cb) + defer eng.Stop() + if err := eng.Watch("game1", saveDir); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(saveDir, "slot1.sav"), []byte("played"), 0o666); err != nil { + t.Fatal(err) + } + select { + case <-inSnapshot: + case <-time.After(15 * time.Second): + t.Fatal("the change was never snapshotted") + } + + stopped := make(chan struct{}) + go func() { + eng.Unwatch("game1") + close(stopped) + }() + time.Sleep(300 * time.Millisecond) // the watch is cancelled; its snapshot still runs + close(release) + <-stopped + time.Sleep(300 * time.Millisecond) + + if n := col.snapshotCount(); n != 1 { + t.Errorf("%d snapshots, want the one being taken kept", n) + } + if n := col.changedCount(); n != 0 { + t.Errorf("the change was announced %d time(s) after its watch was stopped", n) + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 612ebfb6..3e81268f 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -844,6 +844,15 @@ func (e *Engine) handleChange(ctx context.Context, gw *gameWatch) { } e.log("success", fmt.Sprintf("auto-snapshot created for %q", gw.gameID)) + // Stopped while the snapshot was taken — auto-sync switched off, the game + // untracked, the app quitting: the snapshot is kept, as it is the save as + // it changed, but nothing more happens on behalf of a watch nobody wants. + // A snapshot can take seconds, longer than stopping waits (stop), so this + // ran on after the watch was given up and started a sync of a game whose + // auto-sync had just been switched off. + if ctx.Err() != nil { + return + } if e.cb.OnChanged != nil { e.cb.OnChanged(gw.gameID) }