From a8b6c13b0100b28052cf559262b73b03d82f5286 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Fri, 28 Aug 2026 18:55:05 -0400 Subject: [PATCH] Report a crashed service as failed, which OpenRC was never telling us The OpenRC branch of this script had never run anywhere. A guest built to reproduce the host says it was wrong in the way the `failed` state exists to prevent: a service that fell over reported `stopped`, which reads as a decision somebody made. Two causes, both measured rather than reasoned about. `rc-service status` prints `* status: crashed` to **stderr**. The check was `status 2>/dev/null | grep -qw crashed`, which discards precisely the word it is searching for, finds nothing, and falls through to `stopped`. The old comment argued for reading the word rather than the exit code, and that argument was sound except that the code turns out to be specific rather than merely non-zero. So it now reads the code, which says more than the text did: 0 started, 3 stopped, 32 crashed, and 1 for every way the question cannot be answered -- an unknown service, XDG_RUNTIME_DIR unset, or a user softlevel that was never initialised. That last group is a real state and not one of the other three, so it exits non-zero and says so instead of guessing. `set -e` is the second cause, found by running the first fix: every answer except "running" is a non-zero exit, so a bare invocation killed the script before the code could be looked at. It prints nothing and exits 3, which is indistinguishable from a crash of this script itself. Verified in the guest, all four states: not-installed, stopped, failed after a real crash, and a non-zero exit with nothing on stdout when the softlevel is removed. Before the fix the crashed case printed `stopped`. --- server/service | 83 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 59 insertions(+), 24 deletions(-) diff --git a/server/service b/server/service index 7a48358..6b35706 100755 --- a/server/service +++ b/server/service @@ -87,19 +87,27 @@ installed() { esac } -# Whether the service fell over, as opposed to being stopped on purpose. +# What OpenRC's `status` exit code means. Measured on OpenRC 0.63.3 in a +# guest built for the purpose, not read from documentation: # -# OpenRC prints `crashed` *and* exits non-zero for this, so the word is -# read rather than the exit code -- leaning on the code would report -# "couldn't check", which is a different and less useful thing to say. -# The `running` check above stays on its exit code, which already worked -# and does not depend on wording. -crashed() { - case "$MANAGER" in - systemd) systemctl --user --quiet is-failed "$NAME" ;; - openrc) rc-service --user "$NAME" status 2>/dev/null | grep -qw crashed ;; - esac -} +# 0 started 3 stopped 32 crashed +# 1 could not find out +# +# 1 covers every way the question cannot be answered -- an unknown +# service, XDG_RUNTIME_DIR unset, or a user softlevel that was never +# initialised ("openrc did not boot this system"). Distinguishing it is +# the whole reason to read the code rather than the text. +# +# And the text must not be read. OpenRC prints `* status: crashed` to +# **stderr**, so the obvious `status 2>/dev/null | grep -qw crashed` +# throws away precisely the word it is searching for, finds nothing, and +# falls through to `stopped` -- reporting a service that fell over as one +# somebody chose to stop. That was this script's bug until it was +# measured, and the same shape is worth checking wherever a status is +# parsed rather than counted. +OPENRC_STARTED=0 +OPENRC_STOPPED=3 +OPENRC_CRASHED=32 require_binary() { [ -x "$BINARY" ] && return 0 @@ -212,19 +220,46 @@ case "${1:-}" in status) if ! installed; then echo not-installed - elif case "$MANAGER" in - systemd) systemctl --user --quiet is-active "$NAME" ;; - openrc) rc-service --user "$NAME" status >/dev/null 2>&1 ;; - esac then - echo running - elif crashed; then - # Before `stopped`, because a crashed service satisfies - # neither of the other two and would otherwise be reported as - # a state somebody chose. - echo failed - else - echo stopped + exit 0 fi + case "$MANAGER" in + systemd) + if systemctl --user --quiet is-active "$NAME"; then + echo running + # Before `stopped`, because a service that fell over + # satisfies neither of the other two and would otherwise + # be reported as a state somebody chose. + elif systemctl --user --quiet is-failed "$NAME"; then + echo failed + else + echo stopped + fi + ;; + openrc) + # `|| code=$?` rather than a bare call: `set -e` is on, and + # every answer except "running" is a non-zero exit, so a + # plain invocation kills the script before it can say what + # the code meant. + code=0 + rc-service --user "$NAME" status >/dev/null 2>&1 || code=$? + case $code in + "$OPENRC_STARTED") echo running ;; + "$OPENRC_CRASHED") echo failed ;; + "$OPENRC_STOPPED") echo stopped ;; + # Anything else is "could not find out", which is a + # real state and not one of the other three. Saying so + # costs a non-zero exit and buys the caller the truth; + # guessing `stopped` here is what the contract's + # `failed` state exists to stop. + *) + echo "could not ask OpenRC about $NAME -- is" \ + "XDG_RUNTIME_DIR set and the user softlevel" \ + "initialised?" >&2 + exit 1 + ;; + esac + ;; + esac ;; *) echo "usage: $0 install|uninstall|start|stop|restart|status|logs" >&2