feat: lock backups internally so cron needs no flock wrapper

rsyncshot now acquires an exclusive, non-blocking lock (fd 200) before the backup phase, so a slow backup can't overlap the next cron tick. Only real backups lock. Help, list, status, and dryrun don't, so they're never blocked.

The lock file defaults to /run/lock/rsyncshot.lock, a root-only directory that avoids the symlink-truncation risk of a predictable name in world-writable /tmp. It falls back to /var/lock then /tmp, and is configurable via LOCKFILE. It's deliberately a different file from the old /tmp/rsyncshot.lock cron wrappers, so existing wrapped crontabs keep working until the wrapper is removed.

setup() no longer wraps cron entries in flock, since the script locks itself.

2 files changed, 137 insertions, 0 deletions

diff --git a/tests/cases/test_cron.sh b/tests/cases/test_cron.sh
index 5c9bbfa..7e38060 100755
--- a/tests/cases/test_cron.sh
+++ b/tests/cases/test_cron.sh
@@ -119,6 +119,35 @@ EOF
 }
 
 # ------------------------------------------------------------------------------
+# Test: Cron entries no longer wrap rsyncshot in an external flock
+# ------------------------------------------------------------------------------
+# rsyncshot locks internally now, so setup should write bare cron commands.
+test_cron_no_flock_wrapper() {
+    setup_test_env
+    save_crontab
+
+    crontab -r 2>/dev/null || true
+
+    cat > "$TEST_CONFIG_DIR/config" << EOF
+REMOTE_HOST=""
+MOUNTDIR="$TEST_BACKUP_DIR"
+EOF
+    create_test_includes
+    create_test_excludes
+
+    sudo INSTALLHOME="$TEST_CONFIG_DIR" SCRIPTLOC="/tmp/rsyncshot-test" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" setup >/dev/null 2>&1 || true
+
+    local crontab_content
+    crontab_content=$(crontab -l 2>/dev/null)
+
+    restore_crontab
+    teardown_test_env
+
+    assert_not_contains "$crontab_content" "flock" "cron entries should not wrap rsyncshot in flock (internal locking)" || return 1
+    assert_contains "$crontab_content" "rsyncshot" "cron should still contain rsyncshot entries" || return 1
+}
+
+# ------------------------------------------------------------------------------
 # Run tests
 # ------------------------------------------------------------------------------
 run_cron_tests() {
@@ -129,6 +158,7 @@ run_cron_tests() {
     run_test "setup adds cron jobs" test_setup_adds_cron_jobs
     run_test "repeated setup doesn't duplicate entries" test_no_duplicate_cron_entries
     run_test "setup preserves existing cron jobs" test_preserves_existing_cron
+    run_test "cron entries have no flock wrapper" test_cron_no_flock_wrapper
 }
 
 # Run if executed directly
diff --git a/tests/cases/test_locking.sh b/tests/cases/test_locking.sh
new file mode 100644
index 0000000..4667d58
--- /dev/null
+++ b/tests/cases/test_locking.sh
@@ -0,0 +1,107 @@
+#!/usr/bin/env bash
+# ==============================================================================
+# Concurrency Locking Tests
+# ==============================================================================
+# rsyncshot acquires an exclusive lock before the backup phase. These tests hold
+# the lock in the test process (a separate open file description), then run the
+# script with LOCKFILE pointed at the same file, so the script's flock -n fails
+# exactly as a second concurrent backup would.
+
+source "$(dirname "${BASH_SOURCE[0]}")/../lib/test_helpers.sh"
+
+# ------------------------------------------------------------------------------
+# A real backup refuses to start while another holds the lock
+# ------------------------------------------------------------------------------
+test_refuses_when_lock_held() {
+    setup_test_env
+    create_test_config >/dev/null
+    create_test_includes >/dev/null
+    create_test_excludes >/dev/null
+
+    local lock="$TEST_DIR/rsyncshot.lock"
+    exec 9>"$lock"
+    if ! flock -n 9; then
+        echo "FAIL: test could not acquire the lock to set up the scenario"
+        exec 9>&-
+        teardown_test_env
+        return 1
+    fi
+
+    local output rc
+    output=$(INSTALLHOME="$TEST_CONFIG_DIR" LOCKFILE="$lock" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 1 2>&1)
+    rc=$?
+    exec 9>&-   # release the lock for later tests
+
+    assert_exit_code 1 "$rc" "should exit 1 when another instance holds the lock" || { teardown_test_env; return 1; }
+    assert_contains "$output" "already running" "should report the lock contention" || { teardown_test_env; return 1; }
+
+    teardown_test_env
+}
+
+# ------------------------------------------------------------------------------
+# A dryrun is read-only and must NOT be blocked by a held lock
+# ------------------------------------------------------------------------------
+test_dryrun_not_blocked_by_lock() {
+    setup_test_env
+    create_test_config >/dev/null
+    create_test_includes >/dev/null
+    create_test_excludes >/dev/null
+
+    local lock="$TEST_DIR/rsyncshot.lock"
+    exec 9>"$lock"
+    if ! flock -n 9; then
+        echo "FAIL: test could not acquire the lock to set up the scenario"
+        exec 9>&-
+        teardown_test_env
+        return 1
+    fi
+
+    local rc
+    INSTALLHOME="$TEST_CONFIG_DIR" LOCKFILE="$lock" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" dryrun manual 1 >/dev/null 2>&1
+    rc=$?
+    exec 9>&-
+
+    assert_exit_code 0 "$rc" "dryrun should not be blocked by a held lock" || { teardown_test_env; return 1; }
+
+    teardown_test_env
+}
+
+# ------------------------------------------------------------------------------
+# Two sequential real backups both succeed (lock is released on exit)
+# ------------------------------------------------------------------------------
+test_sequential_runs_each_acquire_lock() {
+    setup_test_env
+    create_test_config >/dev/null
+    create_test_includes >/dev/null
+    create_test_excludes >/dev/null
+
+    local lock="$TEST_DIR/rsyncshot.lock"
+    INSTALLHOME="$TEST_CONFIG_DIR" LOCKFILE="$lock" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 2 >/dev/null 2>&1
+    local rc1=$?
+    INSTALLHOME="$TEST_CONFIG_DIR" LOCKFILE="$lock" RSYNCSHOT_SKIP_MOUNT_CHECK=1 "$SCRIPT_PATH" manual 2 >/dev/null 2>&1
+    local rc2=$?
+
+    assert_exit_code 0 "$rc1" "first run should succeed and release the lock" || { teardown_test_env; return 1; }
+    assert_exit_code 0 "$rc2" "second run should acquire the freed lock and succeed" || { teardown_test_env; return 1; }
+
+    teardown_test_env
+}
+
+# ------------------------------------------------------------------------------
+# Run tests
+# ------------------------------------------------------------------------------
+run_locking_tests() {
+    echo ""
+    echo "Running locking tests..."
+    echo "------------------------------------------------------------"
+
+    run_test "refuses to run when the lock is held" test_refuses_when_lock_held
+    run_test "dryrun is not blocked by the lock" test_dryrun_not_blocked_by_lock
+    run_test "sequential runs each acquire the lock" test_sequential_runs_each_acquire_lock
+}
+
+# Run if executed directly
+if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
+    run_locking_tests
+    print_summary
+fi