bisync: fix handling of unreadable lockfiles - fixes #9290

Lockfiles with invalid JSON content caused bisync to fail permanently
because lockFileIsExpired() logged the decode error but still fell
through to the "valid lock file" path with zero-value TimeExpires.

Now when a JSON decode error is detected:
- If --max-lock is set (< basicallyforever): treat garbled lockfile as
  expired, mark listings failed, and proceed (safe assumption: the
  previous bisync run crashed and left garbage).
- If --max-lock is not set (default): log a clear error telling the
  user the lockfile needs manual inspection, and return false.

cmd/bisync/lockfile.go

@@ -98,15 +98,26 @@ func (b *bisyncRun) lockFileIsExpired() bool {
 		rdf, err := os.Open(b.lockFile)
 		b.handleErr(b.lockFile, "error reading lock file", err, true, true)
 		dec := json.NewDecoder(rdf)
+		var decodeErr error
 		for {
 			if err := dec.Decode(&b.lockFileOpt.data); err != nil {
 				if err != io.EOF {
-					fs.Errorf(b.lockFile, "err: %v", err)
+					decodeErr = err
 				}
 				break
 			}
 		}
 		b.handleErr(b.lockFile, "error closing file", rdf.Close(), true, true)
+		if decodeErr != nil {
+			if b.opt.MaxLock < basicallyforever {
+				fs.Infof(b.lockFile, Color(terminal.YellowFg, "Lock file is unreadable (decode error: %v) and --max-lock is set. Treating as expired."), decodeErr)
+				markFailed(b.listing1)
+				markFailed(b.listing2)
+				return true
+			}
+			fs.Errorf(b.lockFile, Color(terminal.RedFg, "Lock file exists, but contents are unreadable. (decode error: %v)"), decodeErr)
+			return false
+		}
 		if !b.lockFileOpt.data.TimeExpires.IsZero() && b.lockFileOpt.data.TimeExpires.Before(time.Now()) {
 			fs.Infof(b.lockFile, Color(terminal.GreenFg, "Lock file found, but it expired at %v. Will delete it and proceed."), b.lockFileOpt.data.TimeExpires)
 			markFailed(b.listing1) // listing is untrusted so force revert to prior (if --recover) or create new ones (if --resync)

cmd/bisync/lockfile_test.go

@@ -0,0 +1,80 @@
+package bisync
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/rclone/rclone/fs"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func newTestLockfileBisyncRun(t *testing.T, lockContent string, maxLock fs.Duration) *bisyncRun {
+	t.Helper()
+	dir := t.TempDir()
+	lockPath := filepath.Join(dir, "test.lck")
+	require.NoError(t, os.WriteFile(lockPath, []byte(lockContent), 0600))
+
+	listing1 := filepath.Join(dir, "listing1")
+	listing2 := filepath.Join(dir, "listing2")
+	require.NoError(t, os.WriteFile(listing1, []byte(""), 0600))
+	require.NoError(t, os.WriteFile(listing2, []byte(""), 0600))
+
+	return &bisyncRun{
+		lockFile: lockPath,
+		opt:      &Options{MaxLock: maxLock},
+		listing1: listing1,
+		listing2: listing2,
+	}
+}
+
+func TestLockfileIsExpired_UnreadableWithMaxLock(t *testing.T) {
+	b := newTestLockfileBisyncRun(t, "not json!!!", fs.Duration(5*time.Minute))
+	assert.True(t, b.lockFileIsExpired(), "unreadable lockfile with --max-lock set should be treated as expired")
+}
+
+func TestLockfileIsExpired_UnreadableWithoutMaxLock(t *testing.T) {
+	b := newTestLockfileBisyncRun(t, "not json!!!", basicallyforever)
+	assert.False(t, b.lockFileIsExpired(), "unreadable lockfile without --max-lock should not be treated as expired")
+}
+
+func TestLockfileIsExpired_ValidExpired(t *testing.T) {
+	data := struct {
+		Session     string
+		PID         string
+		TimeRenewed time.Time
+		TimeExpires time.Time
+	}{
+		Session:     "test",
+		PID:         "12345",
+		TimeRenewed: time.Now().Add(-10 * time.Minute),
+		TimeExpires: time.Now().Add(-5 * time.Minute),
+	}
+	content, err := json.Marshal(data)
+	require.NoError(t, err)
+
+	b := newTestLockfileBisyncRun(t, string(content), fs.Duration(5*time.Minute))
+	assert.True(t, b.lockFileIsExpired(), "valid lockfile with past expiry should be expired")
+}
+
+func TestLockfileIsExpired_ValidNotExpired(t *testing.T) {
+	data := struct {
+		Session     string
+		PID         string
+		TimeRenewed time.Time
+		TimeExpires time.Time
+	}{
+		Session:     "test",
+		PID:         "12345",
+		TimeRenewed: time.Now(),
+		TimeExpires: time.Now().Add(10 * time.Minute),
+	}
+	content, err := json.Marshal(data)
+	require.NoError(t, err)
+
+	b := newTestLockfileBisyncRun(t, string(content), fs.Duration(5*time.Minute))
+	assert.False(t, b.lockFileIsExpired(), "valid lockfile with future expiry should not be expired")
+}

docs/content/bisync.md

@@ -820,6 +820,12 @@ running -- and you can therefore be reasonably sure that any *expired* lock
 file you may find was left there by an interrupted run, not one that is still
 running and just taking awhile.
 
+If a lock file exists but its contents are unreadable (for example, due to an
+incomplete write or disk error), it is treated as expired when `--max-lock` is
+set to a value greater than `0`. If `--max-lock` is `0` or not set, an
+unreadable lock file will produce an error and block future runs until removed
+manually.
+
 If `--max-lock` is `0` or not set, the default is that lock files will never
 expire, and will block future runs (of these same two bisync paths)
 indefinitely.