fix(on-device-install): daemon restart never replaced the running process

Three stacked bugs, found and confirmed on real hardware while
downgrading a speaker: the new binary landed on disk correctly, but
the running service kept reporting the old version indefinitely.

- install.sh called `/etc/init.d/aftertouch start`, not `restart`,
  after installing. start-stop-daemon silently refuses to launch a
  second instance when one is already running, and the init script
  never checked its exit status, so the old process was never
  replaced.

- The init script started the daemon through a `sh -c "exec ... |
  logger"` pipeline, on the assumption that `exec` lets --make-pidfile
  record the daemon's own PID. POSIX forks each side of a pipe into
  its own process, so the wrapper shell (not the daemon) was the one
  actually tracked. `stop` killed the wrapper, which doesn't forward
  SIGTERM to its children, orphaning the real daemon to keep running
  and keep holding :8000 forever.

- Once the wrapper correctly tracked the daemon's own PID, a further
  race surfaced: start-stop-daemon's own "already running?" check
  matched on generic `/bin/sh` identity, so a `restart`'s `start`
  phase could catch the previous wrapper still mid-teardown and
  silently refuse to launch a new one (masked by --quiet, looking
  like a 120s hang).

Fixed by calling `restart` instead of `start` in install.sh, and by
having the wrapper shell record the daemon's real PID itself (via $!)
while keying start-stop-daemon's own check on that same pidfile
instead of process identity.

Verified on hardware: three consecutive restart cycles, each fast,
each with the pidfile matching the live daemon PID and daemon output
flowing through syslog again via logread.

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

scripts/on-device-install/aftertouch

@@ -42,25 +42,67 @@ case "$1" in
 
     mkdir -p "$DATADIR"
 
-    # Pipe stdout + stderr through `logger -t $LOG_TAG` so the
+    # Route stdout + stderr through `logger -t $LOG_TAG` so the
     # daemon's output lands in busybox syslog (bounded ring buffer,
     # never grows on disk). Users diagnose with:
     #
     #   logread        | grep aftertouch | tail -20
     #   logread -f     | grep aftertouch     # live tail
     #
-    # `exec` on the daemon replaces /bin/sh so --make-pidfile records
-    # the daemon's own PID (not the shell wrapper). The `logger`
-    # process sits on the read end of the pipe and exits cleanly
-    # when the daemon dies and closes its end.
+    # This used to be a `--startas "/bin/sh" -- -c "exec $DAEMON | logger"`
+    # pipeline, on the theory that `exec` replaces /bin/sh so --make-pidfile
+    # records the daemon's own PID. That's wrong for a *piped* command:
+    # POSIX requires each side of a pipe to run in its own forked process,
+    # so the top-level /bin/sh forks two children (one execs into the
+    # daemon, one becomes logger) and stays alive itself, blocked in
+    # wait() -- --make-pidfile recorded *that* wrapper's PID, not the
+    # daemon's. `stop` then killed the wrapper, which doesn't forward
+    # SIGTERM to its children, orphaning the real daemon (reparented to
+    # init) to keep running -- and keep holding :8000 -- forever, silently
+    # surviving every later stop/start/restart.
+    #
+    # A first fix attempt dropped the wrapper shell entirely in favor of
+    # `--exec "$DAEMON"` directly, with a plain shell-level `>FIFO`
+    # redirection on the start-stop-daemon invocation. That broke logging
+    # instead: this busybox's `--background` resets the backgrounded
+    # child's own stdio, ignoring the outer redirection, so the daemon's
+    # output never reached the FIFO -- confirmed on hardware (`logger`
+    # exited immediately with nothing to read, `logread` showed nothing
+    # new).
+    #
+    # This version keeps a wrapper shell -- its *own* FIFO redirection,
+    # set up by its own script logic rather than inherited from outside,
+    # isn't affected by whatever --background did to its stdio -- but has
+    # the wrapper record the daemon's real PID itself instead of trusting
+    # --make-pidfile. $! after a single, non-piped backgrounded command is
+    # portably that command's own PID; --make-pidfile can only ever see
+    # whatever process start-stop-daemon directly forked (the wrapper),
+    # never a PID from inside it.
+    LOGFIFO="/tmp/$NAME.fifo"
+    rm -f "$LOGFIFO"
+    mkfifo "$LOGFIFO"
+
+    # --pidfile (without --make-pidfile, since the wrapper writes it itself
+    # once it knows the daemon's real PID) makes start-stop-daemon's own
+    # "already running?" check keyed on *our* pidfile, not on "/bin/sh"
+    # identity. Without this, --startas "/bin/sh" is itself the match
+    # criterion -- and since the wrapper stays alive for the daemon's whole
+    # lifetime (blocked in its own `wait`), and `stop` only confirms the
+    # *daemon* PID died (not that the wrapper has finished tearing down),
+    # a `restart` firing `start` right after `stop` can catch the previous
+    # wrapper still mid-teardown. start-stop-daemon then silently refuses
+    # ("/bin/sh is already running", swallowed by --quiet) while the
+    # script burns its full 120s timeout waiting for a daemon that was
+    # never launched. Confirmed on hardware: a bare
+    # `start-stop-daemon --startas "/bin/sh" -- -c "echo hi"` was refused
+    # with exactly that message while a prior wrapper was still alive.
     start-stop-daemon --start \
         --quiet \
         --pidfile "$PIDFILE" \
         --background \
-        --make-pidfile \
         --chuid "$USER" \
         --startas "/bin/sh" \
-        -- -c "exec \"$DAEMON\" --data-dir '$DATADIR' --record-interactions=false --discovery-interval=60m 2>&1 | logger -t $LOG_TAG"
+        -- -c "logger -t $LOG_TAG <'$LOGFIFO' & \"$DAEMON\" --data-dir '$DATADIR' --record-interactions=false --discovery-interval=60m >'$LOGFIFO' 2>&1 & echo \$! >'$PIDFILE'; wait"
 
     tries=0
     max_tries=60

scripts/on-device-install/install.sh

@@ -138,8 +138,18 @@ mv "$UPDATE_TMP_DIR/init-script" /etc/init.d/aftertouch
 chmod +x /etc/init.d/aftertouch
 update-rc.d aftertouch defaults
 
-echo "Installation complete. Running initial startup..."
-/etc/init.d/aftertouch start
+echo "Installation complete. (Re)starting the service..."
+# Use `restart`, not `start`: if AfterTouch is already running (the normal
+# case for an in-place upgrade or downgrade), `start` calls start-stop-daemon
+# with a pidfile that still points at a live PID. start-stop-daemon then
+# refuses to launch a second instance and exits non-zero -- but this script
+# has no `set -e` here and never checked that exit status, so the old
+# process kept running untouched while the new binary sat unused on disk.
+# The post-install curl check below couldn't catch it either, since the old
+# process kept answering on :8000 throughout. `restart` stops the old
+# process first (a no-op if nothing was running yet, e.g. on a fresh
+# install), guaranteeing the newly-installed binary is the one that starts.
+/etc/init.d/aftertouch restart
 
 /etc/init.d/aftertouch status