fleetd #593 (pid-count half): running_pid() no longer matches the caller
running_pid() was a bare `pgrep -f "$PATTERN"`, which matches ANY process whose full command line contains the pattern text -- including a shell that merely embeds it as literal text (a hand-typed investigation, an ssh-shaped `sh -c '...; ...'`, or a pipeline) rather than being the daemon. That self-match turns a working redeploy into a reported "racing supervisor" failure via assert_single_daemon. pgrep -c does not exist on BSD/macOS, so this can't be fixed by switching flags. running_pid() now keeps pgrep to find candidates (portable), then drops any candidate whose process name (comm) names a shell -- the daemon is always `java`, so a self-matching wrapper of this shape is always excluded while a genuine second daemon-shaped process still counts. assert_single_daemon's die message no longer hands the operator a bare `pgrep -f "$PATTERN"` as remediation -- that was exactly the self-matching invocation -- and now says in words that a pattern can match the caller. Adds three tests: a self-matching wrapper shell must be excluded, a real second daemon-shaped process must still be found, and the die message must not recommend the self-matching command. Verified the first test fails against the pre-fix implementation (confirmed the regression is caught). Leaves instance 1 (the fleetd.out log source, systemd-only) for a Linux host, per the ticket's scope split.
This commit is contained in:
@@ -169,7 +169,37 @@ hash256() {
|
||||
# on PATH). "absent" must never be the answer for a file that exists — that conflation, on Linux,
|
||||
# was the whole defect this ticket fixes.
|
||||
jar_id() { local f="${1:-$JAR}"; [ -f "$f" ] && hash256 "$f" || echo "absent"; }
|
||||
running_pid() { pgrep -f "$PATTERN" || true; }
|
||||
|
||||
# fleetd #593 — `pgrep -f "$PATTERN"` matches ANY process whose full command line CONTAINS the
|
||||
# pattern text, and that is not the same thing as "is the daemon". A shell that merely embeds the
|
||||
# pattern as literal text — a human typing this exact investigation by hand, an ssh-shaped
|
||||
# `sh -c '...; ...'`, a pipeline, or any other non-exec'ing shell that never replaced itself with
|
||||
# the pattern-holding command — still shows up in that match, and it is the INSTRUMENT, not the
|
||||
# daemon. Measured live on this Mac: `sh -c 'echo "target/fleetd.jar" >/dev/null; sleep 30' &`
|
||||
# leaves a real `sh` process alive (it forks for the `sleep`, it does not exec into it) whose own
|
||||
# `ps -o args` is `sh -c echo "target/fleetd.jar" >/dev/null; sleep 30` — `pgrep -f "$PATTERN"`
|
||||
# matches that line right alongside the real `java -jar target/fleetd.jar` process. `pgrep -c`
|
||||
# (an in-one-call count) does not exist on BSD/macOS at all, so this cannot be fixed by switching
|
||||
# pgrep flags — it has to filter what pgrep already found, after the fact, in a way that still
|
||||
# runs on BSD. The daemon is always started as `java -jar target/fleetd.jar`, so its process name
|
||||
# (comm) is always `java`; a self-matching wrapper of this shape is always a shell. So: keep pgrep
|
||||
# to FIND candidates (that part is already portable), then drop any candidate whose `comm` names a
|
||||
# shell — the wrapper is excluded, a genuine second daemon-shaped process (java, or in a test,
|
||||
# anything that is not itself a shell) still counts. `comm` can carry a login shell's leading '-'
|
||||
# (e.g. "-bash") or, on some `ps` builds, a full path — both are stripped before the comparison.
|
||||
running_pid() {
|
||||
local pid comm out=''
|
||||
for pid in $(pgrep -f "$PATTERN" 2>/dev/null || true); do
|
||||
comm="$(ps -o comm= -p "$pid" 2>/dev/null || true)"
|
||||
comm="${comm##*/}"
|
||||
comm="${comm#-}"
|
||||
case "$comm" in
|
||||
sh|bash|zsh|dash|ksh) continue ;;
|
||||
esac
|
||||
out="$out$pid"$'\n'
|
||||
done
|
||||
printf '%s' "$out"
|
||||
}
|
||||
|
||||
# fleetd #493 — three small, independently testable pieces of "never build into the path a
|
||||
# running process holds":
|
||||
@@ -507,8 +537,11 @@ assert_single_daemon() {
|
||||
die "more than one fleetd process is running after this restart (pids: $(printf '%s' "$pids" | tr '\n' ' ')).
|
||||
This is the exact failure a racing supervisor produces: the OLD jar was revived by its
|
||||
supervisor while this script started a NEW copy. Two daemons on one herdr session kill
|
||||
each other's members. Investigate with 'pgrep -f \"$PATTERN\"' and stop the wrong one by
|
||||
hand — do not assume either pid is the one you want."
|
||||
each other's members. Investigate with 'ps -eo pid,comm,args | grep -F \"$PATTERN\"' and
|
||||
check the COMM column of each hit yourself before acting — a bare 'pgrep -f \"$PATTERN\"'
|
||||
(fleetd #593) can match the very shell you type it into, not just the daemon, so it is not
|
||||
safe remediation advice on its own. Stop the wrong one by hand — do not assume either pid
|
||||
is the one you want."
|
||||
fi
|
||||
}
|
||||
|
||||
|
||||
@@ -433,6 +433,67 @@ test_assert_single_daemon_rejects_two_pids() {
|
||||
printf '%s' "$output" | grep -qF '4343' || fail "refusal message does not list the pids it found"
|
||||
}
|
||||
|
||||
# fleetd #593 instance 2 — `running_pid()` used to be a bare `pgrep -f "$PATTERN"`, which matches
|
||||
# ANY process whose full command line contains the pattern TEXT, including a shell that merely
|
||||
# embeds it as literal text rather than being the daemon. Measured live on this Mac: `pgrep -c`
|
||||
# (a one-call count) does not exist on BSD at all, and `bash -c "<single command>"` execs in place
|
||||
# so no parent shell survives to hold the pattern — which is exactly why the defect did not
|
||||
# reproduce from a plain script and needs a wrapper shaped like this instead. A `sh -c '...; ...'`
|
||||
# with MORE THAN ONE statement does not get that exec-in-place treatment: the shell forks a child
|
||||
# for the second statement and stays alive itself, holding the whole `-c` string — pattern text
|
||||
# included — in its own `ps -o args`, for as long as it runs. That is the same shape an
|
||||
# `ssh host "…; …"` wrapper or a hand-typed pipeline leaves behind. Before the fix this test would
|
||||
# have found the wrapper's pid in running_pid()'s output; it must not.
|
||||
test_running_pid_excludes_self_matching_wrapper_shell() {
|
||||
local before after wrapper_pid
|
||||
before="$(running_pid)"
|
||||
sh -c 'echo "target/fleetd.jar" >/dev/null; sleep 20' &
|
||||
wrapper_pid=$!
|
||||
sleep 0.3
|
||||
after="$(running_pid)"
|
||||
kill "$wrapper_pid" 2>/dev/null || true
|
||||
wait "$wrapper_pid" 2>/dev/null || true
|
||||
[ "$after" = "$before" ] \
|
||||
|| fail "running_pid() counted a self-matching wrapper shell (pid $wrapper_pid, holding the pattern as literal text in its own argv, not the daemon): before=[$before] after=[$after]"
|
||||
}
|
||||
|
||||
# fleetd #593 instance 2, the other half — the fix above must not exclude too much. A genuine
|
||||
# daemon-shaped process (comm is not a shell) whose own argv holds the pattern must still be
|
||||
# found. `exec -a` gives a harmless `sleep` an argv[0] containing the pattern without starting a
|
||||
# real JVM: copying a system binary into a scratch path and executing it from there was tried
|
||||
# first and the OS killed it outright (SIGKILL, exit 137 — almost certainly a code-signing check),
|
||||
# so this uses `exec -a` instead, which needs no binary of its own and nothing under a scratch
|
||||
# directory.
|
||||
test_running_pid_still_finds_a_real_second_process() {
|
||||
local before after standin_pid
|
||||
before="$(running_pid)"
|
||||
( exec -a "fleetd-593-test-standin-target/fleetd.jar" sleep 20 ) &
|
||||
standin_pid=$!
|
||||
sleep 0.3
|
||||
after="$(running_pid)"
|
||||
kill "$standin_pid" 2>/dev/null || true
|
||||
wait "$standin_pid" 2>/dev/null || true
|
||||
printf '%s\n' "$after" | grep -qxF "$standin_pid" \
|
||||
|| fail "running_pid() did not find a real second daemon-shaped process (pid $standin_pid) whose own argv holds the pattern: before=[$before] after=[$after]"
|
||||
}
|
||||
|
||||
# fleetd #593 instance 3 — assert_single_daemon's refusal message used to tell the operator to
|
||||
# "Investigate with 'pgrep -f \"\$PATTERN\"'", which — typed by hand or over ssh — is precisely the
|
||||
# self-matching invocation instance 2 above fixes. A source-text check, the same technique
|
||||
# test_no_error_lines_message_gated_by_drain_state uses: this is prose inside a die() call, never
|
||||
# reached by sourcing (the SOURCED guard stops before the main flow, and this text only prints
|
||||
# from inside a call assert_single_daemon makes when it is already refusing).
|
||||
test_die_message_does_not_recommend_bare_pgrep_as_remediation() {
|
||||
local src="$ROOT/scripts/redeploy-fleetd.sh" block bad
|
||||
block="$(grep -A6 -F 'racing supervisor produces' "$src" || true)"
|
||||
[ -n "$block" ] || fail "could not find the assert_single_daemon refusal message in redeploy-fleetd.sh"
|
||||
bad="$(printf '%s' "$block" | grep -F "Investigate with 'pgrep -f" || true)"
|
||||
[ -z "$bad" ] \
|
||||
|| fail "assert_single_daemon's die message still hands the operator a bare 'pgrep -f \"\$PATTERN\"' as remediation (fleetd #593) — that is exactly the self-matching invocation"
|
||||
printf '%s' "$block" | grep -qF 'fleetd #593' \
|
||||
|| fail "assert_single_daemon's die message does not say in words that a pattern can match the caller (fleetd #593)"
|
||||
}
|
||||
|
||||
# fleetd #511 — jar_id()'s no-argument default was unpinned by any test: nothing proved it reports
|
||||
# $JAR (the live path) rather than $JAR_STAGED. Both halves matter, so this pins both: the bare call
|
||||
# must hash the live jar, and an explicit path argument must hash THAT file, not fall back to $JAR.
|
||||
@@ -1894,6 +1955,9 @@ test_require_drivable_supervisor_accepts_known_kinds
|
||||
test_count_daemon_pids
|
||||
test_assert_single_daemon_accepts_one_pid
|
||||
test_assert_single_daemon_rejects_two_pids
|
||||
test_running_pid_excludes_self_matching_wrapper_shell
|
||||
test_running_pid_still_finds_a_real_second_process
|
||||
test_die_message_does_not_recommend_bare_pgrep_as_remediation
|
||||
test_jar_id_defaults_to_live_and_reports_explicit_path
|
||||
test_hash256_computes_a_real_sha256
|
||||
test_jar_id_reports_absent_for_missing_file
|
||||
|
||||
Reference in New Issue
Block a user