diff --git a/Partnership/partnership_manager.sh b/Partnership/partnership_manager.sh index fa6f159..8d88a75 100755 --- a/Partnership/partnership_manager.sh +++ b/Partnership/partnership_manager.sh @@ -79,8 +79,25 @@ # SSH_TIMEOUT on all remote calls # Every ssh/scp call is timeout-protected. No operation hangs on an unreachable peer. # +# No acquire_lock — Deliberate +# This file is dual-role: an executable dispatcher AND a library that +# partnership_offboard.sh and partnership_transfer.sh source with +# PARTNERSHIP_LIB_MODE=1. A top-level acquire_lock would fire for every sourcing +# script, and --check can re-invoke this file as itself (bash "$0" --offboard), +# which a strict lock would deadlock. Concurrency is handled per-write with flock +# instead. Do not "fix" this to match the single-role scripts. +# # flock on state writes -# Prevents concurrent state file corruption from overlapping --check cycles. +# write_state_file() serialises full state-file rewrites, and the --check counter +# updates are flocked separately on their own lock file. The offline counter is a +# read-modify-write: without the lock two overlapping --check cycles both read N and +# both write N+1, silently losing an increment and pushing the auto-offboard threshold +# past its configured window. The last_seen_remote sed is inside the same lock because +# it edits a file write_state_file() rewrites wholesale from other paths. +# +# Note the redirection must sit INSIDE a command substitution — "$( ... ) 201>file" +# attaches the descriptor to the assignment rather than the subshell, and flock then +# fails with "Bad file descriptor" while the unlocked write proceeds anyway. # # SIGTERM trap on grace period sleep # Offboard grace period is interruptible — Ctrl-C aborts cleanly. @@ -1028,17 +1045,33 @@ fi # ============================================================================================== if [[ "$MODE" == "check" ]]; then - # Update last_seen_remote and offline counter based on rsync outcome + # Update last_seen_remote and offline counter based on rsync outcome. + # + # Both branches are flocked. The counter is a read-modify-write, so two overlapping + # --check cycles would otherwise both read N and both write N+1 — silently losing an + # increment and pushing the auto-offboard threshold further out than configured. The + # sed on the state file is included because write_state_file() flocks the same file + # from other code paths, and an unsynchronised sed -i can land mid-rewrite. if [[ "$REMOTE_SEEN" == true ]]; then - echo "0" > "$OFFLINE_COUNTER" - if [[ -f "$LOCAL_STATE_FILE" ]]; then - sed -i "s|^last_seen_remote=.*|last_seen_remote=$(date '+%Y-%m-%d %H:%M:%S')|" \ - "$LOCAL_STATE_FILE" 2>/dev/null - fi + ( + flock -x 201 + echo "0" > "$OFFLINE_COUNTER" + if [[ -f "$LOCAL_STATE_FILE" ]]; then + sed -i "s|^last_seen_remote=.*|last_seen_remote=$(date '+%Y-%m-%d %H:%M:%S')|" \ + "$LOCAL_STATE_FILE" 2>/dev/null + fi + ) 201>"${OFFLINE_COUNTER}.lock" elif [[ "$REMOTE_UNSEEN" == true ]]; then - OFFLINE_COUNT=$(cat "$OFFLINE_COUNTER" 2>/dev/null || echo 0) - OFFLINE_COUNT=$(( OFFLINE_COUNT + 1 )) - echo "$OFFLINE_COUNT" > "$OFFLINE_COUNTER" + # Redirection must live INSIDE the substitution — "$( ... ) 201>file" attaches the fd + # to the assignment, not to the subshell doing the work, and flock then fails with + # "Bad file descriptor" while the increment silently proceeds unlocked. + OFFLINE_COUNT=$( { + flock -x 201 + _c=$(cat "$OFFLINE_COUNTER" 2>/dev/null || echo 0) + _c=$(( _c + 1 )) + echo "$_c" > "$OFFLINE_COUNTER" + echo "$_c" + } 201>"${OFFLINE_COUNTER}.lock" ) # Auto-offboard threshold: threshold_days × 48 intervals/day (every 30min) THRESHOLD_INTERVALS=$(( ${PARTNERSHIP_OFFLINE_THRESHOLD:-30} * 48 ))