fix: prerm kill sweep must not kill its own ancestors

`ps | grep '/usr/bin/tachyon'` matches the hook itself, because
/usr/bin/tachyon is what invoked it. The sweep therefore killed its own
parent, the parent died on SIGKILL, and apk saw
`tachyon-<version>.pre-upgrade: exited with error 247` (128+119) and
reported a failed hook.

The sweep now walks /proc/<pid>/stat to collect its own ancestor chain and
skips those pids.

Holders of procd's serialization lock are also cleared by descriptor rather
than by name. procd_lock() does `exec 1000>` on
/var/lock/procd_tachyon.lock and then `flock 1000` with no -w, so any
background spawn inheriting fd 1000 keeps the lock held. On a live router
the holders were watchdog workers, logread, the zapret2 supervisors, nfqws2
and a stray curl — none of which match a tachyon name pattern. While any of
them lives, every init.d call blocks in flock until the package manager
gives up on the hook.

Same descriptor-based sweep added to install.sh, which faces the same lock
left behind by pre-fix builds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

install.sh

@@ -1206,6 +1206,28 @@ function installer_release_init_lock() {
     for (let pattern in [ "99-tachyon-wan|flock 1000", "/etc/init.d/tachyon" ])
         system("ps 2>/dev/null | grep -E '" + pattern + "' | grep -v grep | awk '{print $1}' | " +
             "while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
+
+    // Killing by name is not enough. procd_lock() does `exec 1000>` on
+    // /var/lock/procd_tachyon.lock and then `flock 1000` with no -w, and every
+    // background spawn that inherits descriptor 1000 keeps the lock held long after
+    // the init script that took it exited. Builds up to 1.2.66 swept /proc/self/fd
+    // only as far as 999 and so missed exactly that descriptor — on a live router
+    // eleven unrelated processes (watchdog workers, logread, the zapret2 supervisors,
+    // nfqws2, a stray curl) were found pinning it. None of them match a tachyon name
+    // pattern, so the holders are cleared by descriptor. While any of them lives, the
+    // old pre-upgrade hook blocks in flock until the package manager kills it and
+    // rolls the whole transaction back.
+    //
+    // The installer's own process tree is skipped: this runs as a child of
+    // /usr/bin/tachyon on an upgrade, and killing an ancestor takes the install down.
+    system("__anc=\" \"; __p=$$; " +
+        "while [ -n \"$__p\" ] && [ \"$__p\" -gt 1 ] 2>/dev/null; do " +
+        "__anc=\"$__anc$__p \"; " +
+        "__p=$(awk '{ sub(/.*\\) /, \"\"); print $2 }' \"/proc/$__p/stat\" 2>/dev/null); done; " +
+        "for __f in /proc/[0-9]*; do [ -e \"$__f/fd/1000\" ] || continue; " +
+        "case \"$(readlink \"$__f/fd/1000\" 2>/dev/null)\" in *procd_tachyon*) " +
+        "__pid=${__f##*/}; case \"$__anc\" in *\" $__pid \"*) continue;; esac; " +
+        "kill -9 \"$__pid\" 2>/dev/null;; esac; done; true");
     return true;
 }
 

tachyon/files/usr/lib/service/package.uc

@@ -100,6 +100,23 @@ function bounded_command_success_from_args(args, seconds) {
     return command_success_from_args(bounded);
 }
 
+// A kill sweep must not take out the process running it. `ps | grep '/usr/bin/tachyon'`
+// matches this very hook — /usr/bin/tachyon is what invoked it — so the naive sweep
+// killed its own parent; the parent died on SIGKILL, and apk saw the pre-upgrade hook
+// exit 247 (128+119) and reported a failed hook. The ancestor chain of the shell doing
+// the sweep is walked out of /proc and skipped. Field 2 after the ")" of /proc/<pid>/stat
+// is the ppid; the comm field is stripped first because it may itself contain spaces.
+function kill_matching_command(grep_args) {
+    return "__anc=\" \"; __p=$$; " +
+        "while [ -n \"$__p\" ] && [ \"$__p\" -gt 1 ] 2>/dev/null; do " +
+        "__anc=\"$__anc$__p \"; " +
+        "__p=$(awk '{ sub(/.*\\) /, \"\"); print $2 }' \"/proc/$__p/stat\" 2>/dev/null); " +
+        "done; " +
+        "ps 2>/dev/null | grep " + grep_args + " | grep -v grep | awk '{print $1}' | " +
+        "while read _pid; do case \"$__anc\" in *\" $_pid \"*) continue;; esac; " +
+        "kill -9 \"$_pid\" 2>/dev/null; done; true";
+}
+
 function path_exists(path) {
     return fs.stat(as_string(path)) != null;
 }
@@ -186,8 +203,19 @@ function prerm_cleanup(action) {
         // to prevent INIT_PATH stop from blocking indefinitely on rc.common flock -w 1000.
         // This has to happen before the status probe below, or that probe becomes the
         // very thing that waits on the lock those processes hold.
-        system("ps 2>/dev/null | grep -E '99-tachyon-wan|flock 1000|init[.]d/tachyon' | awk '{print $1}' | while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
-        system("ps 2>/dev/null | grep -F '/etc/init.d/tachyon' | awk '{print $1}' | while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
+        system(kill_matching_command("-E '99-tachyon-wan|flock 1000|init[.]d/tachyon'"));
+        system(kill_matching_command("-F '/etc/init.d/tachyon'"));
+        // procd leaks its serialization lock: procd_lock() does `exec 1000>` on
+        // /var/lock/procd_tachyon.lock and then `flock 1000` with no -w, so every
+        // background spawn that inherits fd 1000 keeps the lock held after the init
+        // script exits. Builds before this fix swept /proc/self/fd only up to 999 and
+        // missed exactly that descriptor. The holders may therefore be pre-fix
+        // processes with no other reason to die, so they are cleared by descriptor
+        // rather than by name — otherwise INIT_PATH below blocks forever.
+        system("for __f in /proc/[0-9]*; do [ -e \"$__f/fd/1000\" ] || continue; " +
+            "case \"$(readlink \"$__f/fd/1000\" 2>/dev/null)\" in *procd_tachyon*) " +
+            "__pid=${__f##*/}; [ \"$__pid\" = \"$$\" ] || kill -9 \"$__pid\" 2>/dev/null;; " +
+            "esac; done; true");
     }
 
     remember_upgrade_state(action);
@@ -196,8 +224,8 @@ function prerm_cleanup(action) {
         bounded_command_success_from_args([BIN_PATH, "stop"]);
         bounded_command_success_from_args([INIT_PATH, "stop"]);
 
-        // Kill any remaining tachyon processes
-        system("ps 2>/dev/null | grep -F '/usr/bin/tachyon' | awk '{print $1}' | while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
+        // Kill any remaining tachyon processes, minus this hook's own ancestors.
+        system(kill_matching_command("-F '" + BIN_PATH + "'"));
         restore_dnsmasq_if_needed();
         remove_managed_sing_box();
     }
@@ -211,8 +239,8 @@ function postinst_restore() {
 
     if (!PACKAGE_TEST_MODE) {
         // Kill any flock waiters and init.d/tachyon processes that appeared since prerm ran.
-        system("ps 2>/dev/null | grep -E '99-tachyon-wan|flock 1000' | awk '{print $1}' | while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
-        system("ps 2>/dev/null | grep -F '/etc/init.d/tachyon' | awk '{print $1}' | while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
+        system(kill_matching_command("-E '99-tachyon-wan|flock 1000'"));
+        system(kill_matching_command("-F '/etc/init.d/tachyon'"));
     }
     // Start via INIT_PATH. Bounded: a start that hangs must not take the package
     // transaction down with it — the service can be started by hand afterwards,

tests/installer_rollback_detection.sh

@@ -106,6 +106,20 @@ for pattern in '99-tachyon-wan|flock 1000' '/etc/init.d/tachyon'; do
     fail "installer_release_init_lock must clear '$pattern' waiters"
 done
 
+# Killing by name misses the actual holders. procd_lock() does `exec 1000>` on the
+# lock file and `flock 1000` with no -w, so every background spawn inheriting fd 1000
+# keeps the lock. On a live router the holders were watchdog workers, logread, the
+# zapret2 supervisors, nfqws2 and a curl — none of which match a tachyon name.
+grep -Fq '/fd/1000' "$WORK_DIR/lock.uc" ||
+  fail "installer_release_init_lock must clear lock holders by descriptor, not only by name"
+grep -Fq 'procd_tachyon' "$WORK_DIR/lock.uc" ||
+  fail "installer_release_init_lock must only kill holders of Tachyon's own procd lock"
+
+# The installer runs as a child of /usr/bin/tachyon during an upgrade, so a sweep that
+# does not exclude its own ancestors kills the process performing the install.
+grep -Fq '/stat' "$WORK_DIR/lock.uc" ||
+  fail "installer_release_init_lock must walk /proc/<pid>/stat to spare its own ancestors"
+
 # ucode resolves names lexically and does not hoist, so a call placed above the
 # declaration silently becomes null and the lock is never released.
 lock_decl="$(grep -n '^function installer_release_init_lock() {' "$INSTALLER" | head -1 | cut -d: -f1)"

tests/package_lifecycle.sh

@@ -241,4 +241,42 @@ hang_elapsed="$(( $(date +%s) - hang_start ))"
 [ ! -e "$TACHYON_PACKAGE_UPGRADE_STATE" ] ||
   fail "package postinst must clear the upgrade state even when the start times out"
 
+# The kill sweep must not take out the process running it. `ps | grep /usr/bin/tachyon`
+# matches the hook itself — /usr/bin/tachyon is what invoked it — so the naive sweep
+# killed its own parent, the parent died on SIGKILL, and apk saw the pre-upgrade hook
+# exit 247 (128+119) and rolled the whole upgrade back. Verified by running the sweep
+# under a parent whose command line matches the pattern: the parent must survive.
+sweep_parent="$WORK_DIR/usr-bin-tachyon-lookalike"
+cat >"$sweep_parent" <<'SH'
+#!/usr/bin/env bash
+# Command line contains the BIN_PATH the sweep greps for, exactly as the real
+# /usr/bin/tachyon parent does.
+"$@"
+printf '%s\n' parent-survived >>"${TACHYON_SWEEP_LOG:?}"
+SH
+chmod 0755 "$sweep_parent"
+: >"$WORK_DIR/sweep.log"
+: >"$WORK_DIR/rt_tables_sweep"
+TACHYON_SWEEP_LOG="$WORK_DIR/sweep.log" \
+  "$sweep_parent" env \
+  TACHYON_HOOK_COMMAND_TIMEOUT=5 \
+  TACHYON_BIN="$sweep_parent" \
+  TACHYON_INIT="$WORK_DIR/upgrade-init" \
+  TACHYON_DNS_APPLY_UC="$WORK_DIR/missing-dns-apply.uc" \
+  TACHYON_SING_BOX_INIT="$WORK_DIR/missing-sing-box-init" \
+  TACHYON_UCI_STATE_FILE="$WORK_DIR/dont-touch.state" \
+  TACHYON_RT_TABLES="$WORK_DIR/rt_tables_sweep" \
+  TACHYON_SWEEP_LOG="$WORK_DIR/sweep.log" \
+  ucode -L "$TACHYON_LIB" "$PACKAGE_UC" prerm upgrade || true
+grep -Fxq 'parent-survived' "$WORK_DIR/sweep.log" ||
+  fail "package prerm killed its own parent process; apk reports the hook as exit 247 and rolls the upgrade back"
+
+# Holders of procd's lock descriptor are cleared by descriptor, not by name: the
+# processes found pinning it on a router (logread, nfqws2, curl) match no tachyon
+# name pattern, and while any of them lives every init call blocks in flock.
+grep -Fq '/fd/1000' "$PACKAGE_UC" ||
+  fail "package prerm must clear procd lock holders by descriptor"
+grep -Fq 'procd_tachyon' "$PACKAGE_UC" ||
+  fail "package prerm must only kill holders of Tachyon's own procd lock"
+
 printf 'package lifecycle checks passed\n'