From 5e40378072d365c86448298e5052c21762c436cd Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Wed, 2 Sep 2026 17:12:54 -0400 Subject: [PATCH] Fix Android: publish current mtime after the atomic-write rename Edits on the phone were not picked up by syncthing for minutes to hours ("edited a file, but it never synced to my other devices"). Reproduced on device (Pixel 9 Pro, API 35, Syncthing-Fork, shared storage /storage/emulated/0): - The app's temp+rename save lands the new content (inode changes, bytes are correct), yet the file's mtime served through the FUSE layer reverts to the PREVIOUS file's mtime -- exactly, to the nanosecond. MediaProvider's media-scan DB row for the path is not updated when the staging file is renamed into place (logcat: "Database update failed while renaming ..tmp..") and the stale row wins. This reproduces for ANY writer, not just the app (shell temp+rename included). - The fork's inotify watcher ignores these rename-based writes for a long time (a 15:48 edit reached the peer only at 16:39; five consecutive app writes over 6 minutes were never propagated), but it acts on an explicit timestamp update immediately (a touch synced in exactly the 10 s fsWatcherDelayS). Inotify events themselves are delivered fine (verified with an on-device inotify watcher: IN_MOVED_TO arrives). Fix: after the rename in RealFileSystem.WriteFileAtomic, set the file's atime/mtime to now (os.Chtimes, best-effort). utimensat sticks through the FUSE layer and is the update the watcher reacts to. Verified on device after the fix: edit -> peer in ~11-12 s, mtime stays current. Also pins the timestamp contract in TestWriteFileAtomic_PublishesCurrentMtime (mtime is current after a write and advances on rewrite) and documents the invariant in architecture.md 6.5. --- doc/architecture.md | 8 +++++ internal/io/pool/real/filesystem.go | 18 +++++++++++ internal/io/pool/real/filesystem_test.go | 38 ++++++++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/doc/architecture.md b/doc/architecture.md index 4cb426e..68bd078 100644 --- a/doc/architecture.md +++ b/doc/architecture.md @@ -472,6 +472,14 @@ Only the visible byte range is shaped and drawn each frame: (crash leftovers, plus the legacy deterministic "`..tmp`" name). Unique temp names make same-file interleaving structurally impossible even if the serialization regressed. +- **Post-rename timestamp:** immediately after the rename, `WriteFileAtomic` + sets the file's atime/mtime to now (`os.Chtimes`, best-effort). On Android + shared storage the FUSE/MediaProvider layer serves the **previous** file's + mtime after a rename (its media-scan DB row is not updated on rename), and + stat-comparing change detectors (syncthing) then miss the new content + until the next full rescan. The explicit utimensat publishes the current + mtime and is the update the watcher reacts to immediately; it is also the + timestamp contract `TestWriteFileAtomic_PublishesCurrentMtime` pins. - Write failures are tracked per file (`writeFailed`, `retryAttempts`) and retried via `retryChan` (exponential backoff 1 s … 30 s; the timer sends a non-blocking token and the owner re-snapshots at fire time). There is no diff --git a/internal/io/pool/real/filesystem.go b/internal/io/pool/real/filesystem.go index d5a8ac7..813661c 100644 --- a/internal/io/pool/real/filesystem.go +++ b/internal/io/pool/real/filesystem.go @@ -7,6 +7,7 @@ import ( "strconv" "strings" "sync/atomic" + "time" "pad/internal/io/pool/types" ) @@ -109,6 +110,23 @@ func (fs *RealFileSystem) WriteFileAtomic(path string, content []byte) error { return err } + // The Android shared-storage layer (FUSE backed by MediaProvider + // metadata) re-serves the PREVIOUS file's mtime after a rename: its + // media-scan DB row for the path is not updated when the staging file + // is renamed into place (logcat: "Database update failed while + // renaming"). Change detectors that combine inotify events with a + // stat compare (syncthing) then treat the new content as unmodified + // and skip it until the next full rescan — up to an hour of "edited + // but not synced" on a phone. Setting the timestamps explicitly after + // the rename publishes the current mtime through the FUSE layer, and + // the timestamp update is the event the watcher acts on immediately + // (verified on device: a plain rename's mtime is reverted to the old + // file's, while utimensat sticks and triggers the sync at once). + if err := os.Chtimes(fullPath, time.Now(), time.Now()); err != nil { + // The content is already safely in place; the timestamps are a + // best-effort hint for external change detectors, not data. + return nil + } return nil } diff --git a/internal/io/pool/real/filesystem_test.go b/internal/io/pool/real/filesystem_test.go index f772498..fa3db88 100644 --- a/internal/io/pool/real/filesystem_test.go +++ b/internal/io/pool/real/filesystem_test.go @@ -21,6 +21,7 @@ import ( "sync" "sync/atomic" "testing" + "time" ) func randomPayload(t *testing.T, seed int64, size int) []byte { @@ -209,4 +210,41 @@ func TestWriteFileAtomic_StaleTempFileIsConsumed(t *testing.T) { } } +// TestWriteFileAtomic_PublishesCurrentMtime pins the timestamp contract: +// after a successful write the file's mtime is current, and a subsequent +// write ADVANCES it. On Android shared storage a bare rename can leave the +// PREVIOUS file's mtime visible through the FUSE layer (the MediaProvider +// metadata row is not updated on rename), which makes stat-based change +// detectors (syncthing) treat the new content as unmodified until the next +// full rescan. WriteFileAtomic's post-rename Chtimes is what publishes the +// current mtime; if that line is ever removed, this test documents what the +// phone then experiences (it cannot reproduce the clobber itself on ext4). +func TestWriteFileAtomic_PublishesCurrentMtime(t *testing.T) { + d := t.TempDir() + fs := NewRealFileSystem(d) + if err := fs.WriteFileAtomic("/m.txt", []byte("v1")); err != nil { + t.Fatal(err) + } + first, err := os.Stat(filepath.Join(d, "m.txt")) + if err != nil { + t.Fatal(err) + } + if delta := time.Since(first.ModTime()); delta > 30*time.Second || delta < 0 { + t.Fatalf("first write mtime not current: %v (delta %v)", first.ModTime(), delta) + } + // Cross a whole-second boundary: filesystems with 1 s mtime granularity + // (Android FUSE) must still show an advance. + time.Sleep(1100 * time.Millisecond) + if err := fs.WriteFileAtomic("/m.txt", []byte("v2, longer content")); err != nil { + t.Fatal(err) + } + second, err := os.Stat(filepath.Join(d, "m.txt")) + if err != nil { + t.Fatal(err) + } + if !second.ModTime().After(first.ModTime()) { + t.Fatalf("mtime did not advance after rewrite: first=%v second=%v (Android FUSE would report the stale mtime and syncthing would never see the change)", first.ModTime(), second.ModTime()) + } +} + func itoa(n int) string { return strconv.Itoa(n) }