waybeam: do not SIGKILL the encoder when a graceful stop times out

[snokvist]

Follow-up to #2332, on the stop() escalation added in 2f85ed2d.

What and why

That commit fixes a real bug and I am not proposing to undo it. A stop that did not happen must not report OK, because restart then runs start() against a survivor, trips the duplicate-instance check, returns 0, and the requested restart silently never happens. Keeping the honest failure report and restart) stop && start is right.

What this PR changes is the escalation it reaches for. killall -9 is the one thing that must not be done to this daemon on SigmaStar.

waybeam releases kernel-side state on its way out, through MI_SYS_Exit / MI_VENC_DestroyChn. SIGKILL skips that path, so the MI channel and binding slots stay occupied and nothing in userspace can reclaim them. The next start binds a half-open channel: snapshots time out, and the log fills with waiting for encoder data.... In our experience only a power cycle clears it. That is a bad state for an init script to be able to create, because restart is precisely what someone runs remotely when they cannot get to the hardware — the command most likely to be used is the one that can require physical access to undo.

This is not theoretical. We lost a Star6E bench to exactly this on 2026-05-14: a killall -9 on the encoder left the board so that a fresh start logged its init as OK while every snapshot timed out and the main stream sat at waiting for encoder data..., and power-cycling was the only way back. The encoder's own source carries the same warning from another angle, in star6e_audio.c, describing a teardown hang "with the watchdog SIGKILL'ing us before MI_SYS_Exit could run" as the thing to avoid.

There is a second-order cost too, which is what prompted me to send a patch rather than just a comment. The OK (forced) path reports success and lets restart proceed into start() against poisoned kernel state, so the resulting symptom is no encoder data received. That is the same signature a board with a mismatched device tree produces. I spent a long session this week separating those two causes on Infinity6C hardware, and an init script that can manufacture the symptom makes that diagnosis harder for whoever hits it next.

So: keep the fix that mattered, drop the kill. A stop that times out reports the failure and returns 1, which leaves the still-running instance streaming instead of starting a second one on top of wedged state. If the daemon is genuinely hung then the SDK is already stuck, and a reboot is the recovery that returns the board to a known state rather than an unknown one — waybeam's own teardown watchdog already does exactly that when its MI flush wedges, logging teardown wedged for 12s (MI flush D-state) — forcing reboot for deterministic recovery. The daemon has a deterministic escape; the init script's job is to not pre-empt it.

The window also goes from 15s to 30s. waybeam's teardown can legitimately take ~12s when the MI flush is slow, so the old window could fire on a shutdown that was going to succeed. STOP_TIMEOUT_S drives both the loop and the message so the two cannot drift apart.

Evidence

SSC338Q + IMX415, running an image built from master with this package selected (Majestic off, BR2_PACKAGE_WAYBEAM=y). The modified script was run against the live daemon.

Graceful stop — completes in 2s, well inside even the old window, and releases the kernel-side state that SIGKILL would have skipped:

=== pre-state: pid=810
=== stop (timed):
Stopping waybeam: OK
rc=0 elapsed=2s
post-stop pid: none

dmesg:
[CMDQ]Release CMDQ(2) service
client [810] disconnected, module:sensor
client [810] disconnected, module:sys

Those client [810] disconnected lines are the point of the change: that is the MI teardown a SIGKILL never gets to run.

Start after a graceful stop — binds cleanly, no half-open channel:

=== start:
Starting waybeam: OK
rc=0
pid=2846
encoder errors: 0

Restart — the path the original bug made silently no-op — and idempotent start:

=== restart (timed):
Stopping waybeam: OK
Starting waybeam: OK
rc=0 elapsed=2s
pid=3033
encoder errors: 0

=== start when already running:
waybeam already running
rc=0

Stream verified after the restart, at the mode the config selects, via /api/v1/snapshot.jpg:

1920x1080  mean luma 87.64  stddev 14.12

which matches the pre-test reading of 87.84 on the same scene, so the board came back exactly as it was.

sh -n passes on the script.

What I did not test

I have not exercised the timeout branch itself on hardware, because producing a genuinely wedged teardown on demand means deliberately wedging a board, and I did not want to do that to a working one to prove a negative. The graceful path above is what I can demonstrate. The claim behind the change — that SIGKILL leaves unreclaimable MI state — rests on the 2026-05-14 incident and on the encoder's own source comment, not on a reproduction in this PR. If you would rather see the failure branch exercised directly I am happy to do it on a spare board and post what the SIGKILL path actually leaves behind; say the word.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

waybeam: avoid SIGKILL during timed-out graceful stops

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Extends waybeam's graceful shutdown window from 15 to 30 seconds.
• Removes SIGKILL escalation to prevent unrecoverable SigmaStar encoder state.
• Keeps timeout failures nonzero, ensuring restart never starts over a surviving daemon.

Diagram

sequenceDiagram
    actor User as Operator
    participant Init as Init Script
    participant WB as waybeam
    participant MI as SigmaStar MI
    User->>Init: restart
    Init->>WB: SIGTERM
    WB->>MI: release resources
    loop Up to 30s
        Init->>WB: check process
    end
    alt Graceful exit
        Init->>WB: start new instance
        Init-->>User: success
    else Timeout
        Init-->>User: failure
        Note over Init,WB: No SIGKILL; restart blocked
    end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Reboot automatically on timeout

• ➕ Restores a known hardware state when the SDK is genuinely wedged.
• ➕ Provides deterministic remote recovery without requiring a second operator action.
• ➖ Could unexpectedly reboot a camera during a merely slow shutdown.
• ➖ Duplicates waybeam's teardown watchdog and makes ordinary stop operations disruptive.

Recommendation: Keep the PR's fail-safe approach: wait with sufficient headroom, report failure honestly, and block restart without issuing SIGKILL. Automatic reboot is better left to waybeam's existing watchdog or an explicit operator decision because the init script cannot reliably distinguish a permanent SDK wedge from delayed teardown.

Files changed (1) +22 / -11

Bug fix (1) +22 / -11

S95waybeamPrevent forced termination from wedging SigmaStar encoder state +22/-11

Prevent forced termination from wedging SigmaStar encoder state

• Introduces a shared 30-second graceful-stop timeout and uses it for polling and diagnostics. Removes SIGKILL escalation so a timeout returns failure, preserves the existing process, and prevents restart from launching over unreleased SigmaStar MI resources.

general/package/waybeam/files/S95waybeam

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[snokvist]

Withdrawing this patch — we'll carry it in our own fork rather than ask upstream to take it.

To be clear about what is and isn't being withdrawn: the concern itself stands, and I'd leave the note in #2332 (#issuecomment-5466202529) for your consideration. killall -9 on this daemon skips MI_SYS_Exit/MI_VENC_DestroyChn, so the MI channel and binding slots stay occupied and the next start binds a half-open channel that reports no encoder data received until the board is power-cycled. If you'd rather keep the escalation, it's worth knowing that's the failure it can produce, and that the symptom is indistinguishable from a mismatched device tree.

No action needed from anyone here — closing to keep your queue clean.