fix: close procd lock descriptor 1000 in background spawns
procd_lock() in /lib/functions/procd.sh does `exec 1000>/var/lock/procd_<svc>.lock`
followed by `flock 1000` with no -w timeout, so descriptor 1000 IS the procd
serialization lock. The spawn prologue capped its /proc/self/fd sweep at 999 and
therefore skipped exactly that descriptor: every background spawn inherited the
held lock and kept it alive long after the init script that took it had exited.
On the router eleven processes were found pinning /var/lock/procd_tachyon.lock
that way — watchdog workers, logread -f, both zapret2 supervisors and their sh
wrappers, both nfqws2 instances, the telegram worker and a stray curl. With the
lock permanently held, every later /etc/init.d/tachyon call blocked forever in
flock. That is what hung package_prerm until apk killed the pre-upgrade hook and
rolled the whole transaction back, leaving the old build installed and
__COMPILED_COMMIT_SHA__ unsubstituted in the installed constants module.
Killing the flock processes could not fix it: the descriptor lives on in every
inheriting child.
Raise the ceiling past 1000 in all three copies of the prologue (core/common.uc
and the two early-boot duplicates that deliberately carry their own helpers).
tests/background_spawn_fds.sh gains a structural check that every __tfd ceiling
covers 1000 — the runtime probe only runs on a shell that can close high
descriptors, and dash cannot, so CI would never reach it — plus a runtime case
asserting fd 1000 does not survive the prologue where the shell allows it.
Verified on the router: lock holders 0, package_prerm no longer blocks,
the 1.2.66 rebuild installs and constants.uc reports 2fc57d38.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
2fc57d383d
commit
00a5fec0c0
@@ -192,9 +192,21 @@ function command_from_args(args) {
|
|||||||
// The glob's own descriptor may appear in the listing and close underneath the
|
// The glob's own descriptor may appear in the listing and close underneath the
|
||||||
// loop; `|| true` keeps that from ending it. /proc is always mounted on OpenWrt.
|
// loop; `|| true` keeps that from ending it. /proc is always mounted on OpenWrt.
|
||||||
//
|
//
|
||||||
|
// The ceiling has to sit above 1000, not below it. procd_lock() in
|
||||||
|
// /lib/functions/procd.sh does `exec 1000>/var/lock/procd_<svc>.lock` and then
|
||||||
|
// `flock 1000` with no -w, so descriptor 1000 IS the procd serialization lock.
|
||||||
|
// A ceiling of 999 skipped exactly that descriptor: every background spawn
|
||||||
|
// inherited the held lock, and the lock outlived the init script that took it —
|
||||||
|
// watchdog workers, logread -f, the zapret2 supervisors, nfqws2, the telegram
|
||||||
|
// worker and stray curls were all found pinning /var/lock/procd_tachyon.lock.
|
||||||
|
// With the lock never released, any later /etc/init.d/tachyon call blocks
|
||||||
|
// forever in flock, which is how package_prerm hung until apk killed it and
|
||||||
|
// rolled the upgrade back. Killing the flock processes cannot help while an
|
||||||
|
// inheriting child still holds the descriptor.
|
||||||
|
//
|
||||||
// stdin/stdout/stderr are left alone — callers redirect those themselves.
|
// stdin/stdout/stderr are left alone — callers redirect those themselves.
|
||||||
function close_inherited_fds() {
|
function close_inherited_fds() {
|
||||||
return "if ( eval \"exec 10<&-\" ) 2>/dev/null; then __tfd=999; else __tfd=9; fi; " +
|
return "if ( eval \"exec 10<&-\" ) 2>/dev/null; then __tfd=1048576; else __tfd=9; fi; " +
|
||||||
"for f in /proc/self/fd/*; do i=${f##*/}; case $i in 0|1|2) continue;; esac; " +
|
"for f in /proc/self/fd/*; do i=${f##*/}; case $i in 0|1|2) continue;; esac; " +
|
||||||
"[ \"$i\" -le $__tfd ] 2>/dev/null || continue; " +
|
"[ \"$i\" -le $__tfd ] 2>/dev/null || continue; " +
|
||||||
"eval \"exec $i<&-\" 2>/dev/null || true; done; ";
|
"eval \"exec $i<&-\" 2>/dev/null || true; done; ";
|
||||||
|
|||||||
@@ -50,9 +50,9 @@ function command_from_args(args) {
|
|||||||
// Local copy of core/common.uc's background_command(). This module runs on the
|
// Local copy of core/common.uc's background_command(). This module runs on the
|
||||||
// early-boot path and deliberately carries its own helpers rather than pulling
|
// early-boot path and deliberately carries its own helpers rather than pulling
|
||||||
// in core.common; see the rationale there for why the descriptors have to be
|
// in core.common; see the rationale there for why the descriptors have to be
|
||||||
// closed by hand.
|
// closed by hand — including why the ceiling must stay above procd's fd 1000.
|
||||||
function background_command(command) {
|
function background_command(command) {
|
||||||
return "{ if ( eval \"exec 10<&-\" ) 2>/dev/null; then __tfd=999; else __tfd=9; fi; " +
|
return "{ if ( eval \"exec 10<&-\" ) 2>/dev/null; then __tfd=1048576; else __tfd=9; fi; " +
|
||||||
"for f in /proc/self/fd/*; do i=${f##*/}; case $i in 0|1|2) continue;; esac; " +
|
"for f in /proc/self/fd/*; do i=${f##*/}; case $i in 0|1|2) continue;; esac; " +
|
||||||
"[ \"$i\" -le $__tfd ] 2>/dev/null || continue; " +
|
"[ \"$i\" -le $__tfd ] 2>/dev/null || continue; " +
|
||||||
"eval \"exec $i<&-\" 2>/dev/null || true; done; " +
|
"eval \"exec $i<&-\" 2>/dev/null || true; done; " +
|
||||||
|
|||||||
@@ -87,9 +87,9 @@ function command_from_args(args) {
|
|||||||
// Local copy of core/common.uc's background_command(). This module is loaded by
|
// Local copy of core/common.uc's background_command(). This module is loaded by
|
||||||
// the early-boot path and deliberately carries its own helpers rather than
|
// the early-boot path and deliberately carries its own helpers rather than
|
||||||
// pulling in core.common; see the rationale there for why the descriptors have
|
// pulling in core.common; see the rationale there for why the descriptors have
|
||||||
// to be closed by hand.
|
// to be closed by hand — including why the ceiling must stay above procd's fd 1000.
|
||||||
function background_command(command) {
|
function background_command(command) {
|
||||||
return "{ if ( eval \"exec 10<&-\" ) 2>/dev/null; then __tfd=999; else __tfd=9; fi; " +
|
return "{ if ( eval \"exec 10<&-\" ) 2>/dev/null; then __tfd=1048576; else __tfd=9; fi; " +
|
||||||
"for f in /proc/self/fd/*; do i=${f##*/}; case $i in 0|1|2) continue;; esac; " +
|
"for f in /proc/self/fd/*; do i=${f##*/}; case $i in 0|1|2) continue;; esac; " +
|
||||||
"[ \"$i\" -le $__tfd ] 2>/dev/null || continue; " +
|
"[ \"$i\" -le $__tfd ] 2>/dev/null || continue; " +
|
||||||
"eval \"exec $i<&-\" 2>/dev/null || true; done; " +
|
"eval \"exec $i<&-\" 2>/dev/null || true; done; " +
|
||||||
|
|||||||
@@ -119,6 +119,47 @@ if [ "$closes_high" = yes ]; then
|
|||||||
|| fail "background_command left $closed descriptors open in the child; expected the standard three"
|
|| fail "background_command left $closed descriptors open in the child; expected the standard three"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# ── the fd ceiling must sit above procd's lock descriptor ─────────────────────
|
||||||
|
# Structural, because the runtime check below only runs on a shell that can close
|
||||||
|
# high descriptors — dash cannot, so CI would never reach it. Every copy of the
|
||||||
|
# prologue (core/common.uc plus the two early-boot duplicates) must agree.
|
||||||
|
#
|
||||||
|
# Matched per occurrence, not per line: one line carries both the real ceiling
|
||||||
|
# and the dash fallback `__tfd=9`, so a line-wise filter would discard both.
|
||||||
|
ceilings="$(grep -rho '__tfd=[0-9]*' "$LIB_DIR" | sort -u | grep -v '^__tfd=9$' || true)"
|
||||||
|
[ -n "$ceilings" ] || fail "no fd ceiling found; the spawn prologue lost its __tfd assignment"
|
||||||
|
for entry in $ceilings; do
|
||||||
|
value="${entry#__tfd=}"
|
||||||
|
[ "$value" -gt 1000 ] 2>/dev/null \
|
||||||
|
|| fail "fd ceiling $value does not cover procd lock descriptor 1000 (procd_lock does exec 1000> then flock 1000)"
|
||||||
|
done
|
||||||
|
|
||||||
|
# ── descriptor 1000 is the procd lock and must not survive the prologue ───────
|
||||||
|
# procd_lock() in /lib/functions/procd.sh does `exec 1000>/var/lock/procd_<svc>.lock`
|
||||||
|
# then `flock 1000` with no -w. A ceiling of 999 skipped exactly that descriptor,
|
||||||
|
# so every background spawn inherited the held lock and outlived the init script
|
||||||
|
# that took it — after which any /etc/init.d/tachyon call blocked in flock
|
||||||
|
# forever, which is how package_prerm hung until apk rolled the upgrade back.
|
||||||
|
# Killing the flock processes cannot help while an inheriting child holds the fd.
|
||||||
|
if [ "$closes_high" = yes ]; then
|
||||||
|
LOCK_OUT="$(mktemp "${TMPDIR:-/tmp}/tachyon_spawn_lock.XXXXXX")"
|
||||||
|
LOCK_FILE="$(mktemp "${TMPDIR:-/tmp}/tachyon_spawn_lockfile.XXXXXX")"
|
||||||
|
trap 'rm -f "$SPAWN_UC" "$COUNT_OUT" "$LOCK_OUT" "$LOCK_FILE"' EXIT
|
||||||
|
|
||||||
|
cat > "$SPAWN_UC" <<UCODE
|
||||||
|
let common = require("core.common");
|
||||||
|
print(common.background_command("{ ls /proc/self/fd | grep -c '^1000\$' || true; } >>$LOCK_OUT"), "\n");
|
||||||
|
UCODE
|
||||||
|
|
||||||
|
lock_command="$(ucode -L "$LIB_DIR" "$SPAWN_UC")"
|
||||||
|
# Opened the way procd opens it, in this shell, so the child inherits it.
|
||||||
|
( eval "exec 1000>\"\$LOCK_FILE\""; eval "$lock_command"; sleep 0.4 )
|
||||||
|
|
||||||
|
lock_seen="$(tr -d ' \n' < "$LOCK_OUT")"
|
||||||
|
[ "$lock_seen" = 0 ] \
|
||||||
|
|| fail "background_command left procd's lock descriptor 1000 open in the child (saw '$lock_seen'); the fd ceiling must sit above 1000"
|
||||||
|
fi
|
||||||
|
|
||||||
# ── the pid-carrying form reports the daemon, not a wrapper ───────────────────
|
# ── the pid-carrying form reports the daemon, not a wrapper ───────────────────
|
||||||
# `exec` has to replace the subshell, or `$!` names a shell that exits at once
|
# `exec` has to replace the subshell, or `$!` names a shell that exits at once
|
||||||
# and the recorded pid belongs to nothing. Checked against a real spawn: the
|
# and the recorded pid belongs to nothing. Checked against a real spawn: the
|
||||||
|
|||||||
Reference in New Issue
Block a user