diff --git a/CHANGELOG.md b/CHANGELOG.md index 2211ca8..d18033f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,9 @@ dead on the first broken app and said almost nothing about the rest. (`.dockerdance.lock`), so another local user can no longer pre-create the predictable name and permanently lock the tool. It records the owning pid, and a lock whose process is gone clears itself instead of demanding manual - removal after a crash or reboot. + removal after a crash or reboot. Liveness is checked with `ps` rather than + `kill -0`, which cannot see another user's process and would otherwise + declare their running job dead and take the lock from under it. - **Webhook payloads escape their content.** An app name or error message containing a quote, backslash, tab or newline no longer breaks (or rewrites) the JSON sent to `NOTIFY_WEBHOOK`. @@ -56,8 +58,9 @@ dead on the first broken app and said almost nothing about the rest. many older archives exist. - **Releases carry a checksum, and `update-self` verifies it.** The release workflow publishes `manage.sh.sha256` beside the notes; `update-self` - checks the downloaded script against it before installing (releases - without one - everything before 0.4.1 - still install as before). + checks the downloaded script against it before installing (0.4.0 is the + first release to carry one; older releases without it still install as + before). `update-self` also now tells a GitHub rate-limit apart from being offline. - **A test suite.** `tests/run-tests.sh` drives `manage.sh` against a stub docker (no daemon needed) and asserts on exit codes, output and the exact @@ -106,6 +109,11 @@ dead on the first broken app and said almost nothing about the rest. - A `PARALLEL_PULLS` of `0`, a negative or a typo left the pull orchestrator with no slot it could ever fill, hanging the run; it now falls back to the default of 3. +- A lock directory with no readable pid inside - left by a run killed between + creating the lock and recording its owner, or by a full disk - made every + state-changing command exit 1 with no output whatsoever, and cut `doctor`'s + report off mid-way. Such a lock is now reported (and left alone, since + there is no owner to judge) instead of aborting the script. ## [0.3.0] - 2026-07-13 diff --git a/docker_volumes/manage.sh b/docker_volumes/manage.sh index c502ed8..030b82f 100755 --- a/docker_volumes/manage.sh +++ b/docker_volumes/manage.sh @@ -226,24 +226,45 @@ run_step() { return 0 } +#Is process $1 still running? ps is asked first because it can see other +#users' processes: kill -0 only reports "not permitted" for those, which +#would make someone else's live run look dead and let us steal their lock. +#Where ps has no -p (busybox) this falls back to kill -0. +pid_alive() { + ps -p "$1" >/dev/null 2>&1 && return 0 + kill -0 "$1" 2>/dev/null +} + +#Read the pid recorded in the lock, or "" when there isn't a readable one. +#The `|| true` matters: an unguarded assignment from a failing command +#substitution aborts the whole script under set -e, and acquire_lock is +#called as the last command of an AND-OR list, where errexit still applies. +lock_pid() { + lp=$(cat "$LOCK_DIR/pid" 2>/dev/null) || lp="" + printf '%s' "$lp" +} + acquire_lock() { [ -n "$HAVE_LOCK" ] && return 0 if ! mkdir "$LOCK_DIR" 2>/dev/null; then #A lock whose recorded process is gone is left over from a crash or a - #reboot - clear it and try once more. (kill -0 can't tell a foreign - #user's live process from a dead one, but the lock sits inside this - #user's own docker_volumes folder, so that ambiguity doesn't arise.) - al_pid=$(cat "$LOCK_DIR/pid" 2>/dev/null) - if [ -n "$al_pid" ] && ! kill -0 "$al_pid" 2>/dev/null; then + #reboot - clear it and try once more. A lock with no readable pid can't + #be judged, so it's left alone and reported rather than stolen. + al_pid=$(lock_pid) + if [ -n "$al_pid" ] && ! pid_alive "$al_pid"; then warn "Clearing a stale lock left by exited run (pid $al_pid)" - rm -rf "$LOCK_DIR" + rm -rf "$LOCK_DIR" 2>/dev/null || true fi if ! mkdir "$LOCK_DIR" 2>/dev/null; then - error "Another manage.sh run is active here (lock: $LOCK_DIR, pid ${al_pid:-unknown})." + if [ -n "$al_pid" ]; then + error "Another manage.sh run is active here (lock: $LOCK_DIR, pid $al_pid)." + else + error "A lock with no owner recorded is in the way: $LOCK_DIR - remove it if no run is active." + fi exit 1 fi fi - echo "$$" >"$LOCK_DIR/pid" + echo "$$" >"$LOCK_DIR/pid" 2>/dev/null || true HAVE_LOCK=1 } @@ -1058,7 +1079,7 @@ backup_app() { #Newest-first list of $1's own backup archives. The name after the app part #must be exactly a date (with an optional _HHMMSS), in either the current -#'app_YYYY-MM-DD' form or the pre-0.4.1 'appYYYY-MM-DD' form - so an app +#'app_YYYY-MM-DD' form or the pre-0.4.0 'appYYYY-MM-DD' form - so an app #whose name extends another's (vault / vault2) never matches its neighbour's #archives. A bare glob did, which let BACKUP_KEEP prune the wrong app's #backups and restore pick the wrong archive. @@ -1401,11 +1422,13 @@ run_doctor() { fi fi if [ -d "$LOCK_DIR" ]; then - dr_lockpid=$(cat "$LOCK_DIR/pid" 2>/dev/null) - if [ -n "$dr_lockpid" ] && kill -0 "$dr_lockpid" 2>/dev/null; then + dr_lockpid=$(lock_pid) + if [ -n "$dr_lockpid" ] && pid_alive "$dr_lockpid"; then dwarn "A run lock is held by an active run (pid $dr_lockpid)" - else + elif [ -n "$dr_lockpid" ]; then dwarn "A stale run lock exists ($LOCK_DIR) - the next state-changing command will clear it" + else + dwarn "A lock with no owner recorded exists ($LOCK_DIR) - remove it if no run is active" fi else dok "No stale run lock" @@ -1491,7 +1514,7 @@ update_self() { error "The downloaded script failed a syntax check - not installing it." exit 1 fi - #Releases from v0.4.1 publish a checksum next to the script; verify when + #Releases from v0.4.0 publish a checksum next to the script; verify when #it's there (and this host can hash). Older releases simply don't have one. expected_sha=$(fetch_url "https://github.com/$SELF_REPO/releases/download/$latest_tag/manage.sh.sha256" 2>/dev/null | awk 'NR==1{print $1}') || expected_sha="" if [ -n "$expected_sha" ]; then diff --git a/tests/run-tests.sh b/tests/run-tests.sh index feeff2c..9e159a1 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -170,7 +170,7 @@ assert_no_action "UP:alpha" begin "archives: prefix apps (vault/vault2) never cross - prune and restore" apps vault vault2 mkdir -p "$SANDBOX/backup" -#vault's own legacy no-separator archive (pre-0.4.1), and vault2's newer one +#vault's own legacy no-separator archive (pre-0.4.0), and vault2's newer one echo v1 | tar -cjf "$SANDBOX/backup/vault2026-08-01.tar.bz2" -T /dev/null 2>/dev/null || : >"$SANDBOX/backup/vault2026-08-01.tar.bz2" sleep 1 : >"$SANDBOX/backup/vault22026-08-03.tar.bz2" @@ -283,6 +283,27 @@ assert_out "unhealthy: beta" assert_out "Updated 1 of 2 apps" assert_action "UP:beta" #it did start - health is what failed +begin "lock: a pid-less lock is reported, never silently fatal" +#A run killed between mkdir and the pid write (or a full disk) leaves a lock +#with no owner recorded. That must not abort the script under set -e with no +#output at all, and must not be stolen either - we can't tell if it's live. +apps alpha +printf 'alpha\n' >"$DD_STATE/up" +mkdir "$SANDBOX/.dockerdance.lock" +run stop alpha +assert_status 1 +assert_out "no owner recorded" +assert_no_action "STOP:alpha" +#doctor must still print its whole report rather than stopping mid-way +run doctor +assert_status 0 +assert_out "no owner recorded" +assert_out "settings:" #the section printed after the lock check +rmdir "$SANDBOX/.dockerdance.lock" +run stop alpha #and the folder works again once cleared +assert_status 0 +assert_action "STOP:alpha" + begin "missing app folder: reported, run continues" apps alpha gamma