From 5e95877840ce558f2b1e2b7438e8eeaf94e569a1 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Sun, 14 Mar 2021 12:17:29 +0000
Subject: [PATCH] vfs: fix modtime set if --vfs-cache-mode writes/full and no
 write

When using --vfs-cache-mode writes or full if a file was opened for
write intent, the modtime was set and the file was closed without
being modified the modtime would never be written back to storage.

The sequence of events

- app opens file with write intent
- app does set modtime
- rclone sets the modtime on the cache file, but not the remote file
  because it is open for write and can't be set yet
- app closes the file without changing it
- rclone doesn't upload the file because the file wasn't changed so
  the modtime doesn't get updated

This fixes the problem by making sure any unapplied modtime changes
are applied even if the file is not modified when being closed.

Fixes #4795
---
 vfs/file.go       |  7 +++++
 vfs/file_test.go  | 66 ++++++++++++++++++++++++++++++++++++++++++++---
 vfs/read_write.go |  3 +++
 3 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/vfs/file.go b/vfs/file.go
index 4fa6cf17f..3a829adf6 100644
--- a/vfs/file.go
+++ b/vfs/file.go
@@ -424,6 +424,13 @@ func (f *File) _applyPendingModTime() error {
 	return nil
 }
 
+// Apply a pending mod time
+func (f *File) applyPendingModTime() error {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return f._applyPendingModTime()
+}
+
 // _writingInProgress returns true of there are any open writers
 // Call with read lock held
 func (f *File) _writingInProgress() bool {
diff --git a/vfs/file_test.go b/vfs/file_test.go
index 5c226ad5b..a23b5cef1 100644
--- a/vfs/file_test.go
+++ b/vfs/file_test.go
@@ -88,17 +88,55 @@ func TestFileMethods(t *testing.T) {
 	assert.Equal(t, vfs, file.VFS())
 }
 
-func TestFileSetModTime(t *testing.T) {
-	r, vfs, file, file1, cleanup := fileCreate(t, vfscommon.CacheModeOff)
+func testFileSetModTime(t *testing.T, cacheMode vfscommon.CacheMode, open bool, write bool) {
+	if !canSetModTimeValue {
+		t.Skip("can't set mod time")
+	}
+	r, vfs, file, file1, cleanup := fileCreate(t, cacheMode)
 	defer cleanup()
 	if !canSetModTime(t, r) {
 		t.Skip("can't set mod time")
 	}
 
-	err := file.SetModTime(t2)
+	var (
+		err      error
+		fd       Handle
+		contents = "file1 contents"
+	)
+	if open {
+		// Open with write intent
+		if cacheMode != vfscommon.CacheModeOff {
+			fd, err = file.Open(os.O_WRONLY)
+			if write {
+				contents = "hello contents"
+			}
+		} else {
+			// Can't write without O_TRUNC with CacheMode Off
+			fd, err = file.Open(os.O_WRONLY | os.O_TRUNC)
+			if write {
+				contents = "hello"
+			} else {
+				contents = ""
+			}
+		}
+		require.NoError(t, err)
+
+		// Write some data
+		if write {
+			_, err = fd.WriteString("hello")
+			require.NoError(t, err)
+		}
+	}
+
+	err = file.SetModTime(t2)
 	require.NoError(t, err)
 
-	file1.ModTime = t2
+	if open {
+		require.NoError(t, fd.Close())
+		vfs.WaitForWriters(waitForWritersDelay)
+	}
+
+	file1 = fstest.NewItem(file1.Path, contents, t2)
 	fstest.CheckItems(t, r.Fremote, file1)
 
 	vfs.Opt.ReadOnly = true
@@ -106,6 +144,26 @@ func TestFileSetModTime(t *testing.T) {
 	assert.Equal(t, EROFS, err)
 }
 
+// Test various combinations of setting mod times with and
+// without the cache and with and without opening or writing
+// to the file.
+//
+// Each of these tests a different path through the VFS code.
+func TestFileSetModTime(t *testing.T) {
+	for _, cacheMode := range []vfscommon.CacheMode{vfscommon.CacheModeOff, vfscommon.CacheModeFull} {
+		for _, open := range []bool{false, true} {
+			for _, write := range []bool{false, true} {
+				if write && !open {
+					continue
+				}
+				t.Run(fmt.Sprintf("cache=%v,open=%v,write=%v", cacheMode, open, write), func(t *testing.T) {
+					testFileSetModTime(t, cacheMode, open, write)
+				})
+			}
+		}
+	}
+}
+
 func fileCheckContents(t *testing.T, file *File) {
 	fd, err := file.Open(os.O_RDONLY)
 	require.NoError(t, err)
diff --git a/vfs/read_write.go b/vfs/read_write.go
index b16eb663f..50697ce3e 100644
--- a/vfs/read_write.go
+++ b/vfs/read_write.go
@@ -158,6 +158,9 @@ func (fh *RWFileHandle) close() (err error) {
 	if fh.opened {
 		err = fh.item.Close(fh.file.setObject)
 		fh.opened = false
+	} else {
+		// apply any pending mod times if any
+		_ = fh.file.applyPendingModTime()
 	}
 
 	if !fh.readOnly() {