majestic: make S95majestic init script independent of a pidfile

[zavaruev]

Summary

Fix the S95majestic init script so a camera whose majestic died (OOM-kill, kill -9, power glitch) can actually be revived by the common crond watchdog line (* * * * * pgrep majestic || /etc/init.d/S95majestic start) and by repeated S95majestic start calls in general.

Problem

On an OpenIPC SSC335DE camera (52 MB RAM) majestic was OOM-killed and stayed dead for 13+ minutes even though crond executed the watchdog every minute. The camera went fully offline (RTSP dead, no audio, no restart).

Reproduction with the stock script:

• kill -9 a healthy majestic
• wait two minutes: majestic stays dead, while cron.info in the log shows the watchdog line executing every minute
• /var/run/majestic.pid ends up containing pids that never correspond to a live majestic (observed 456, 1355, 1843, 1856, 1870 vs. real daemon pids 4032, 1156, 1546, ...)

Root cause

start()/stop() rely on start-stop-daemon -b -m -S -p $PIDFILE -x majestic, which has two problems on busybox 1.36 (see debianutils/start_stop_daemon.c):

1. The pidfile becomes the single source of truth about "is it running" -- and it gets corrupt. With -b, busybox start-stop-daemon double-forks and the pidfile is written by the final child right before exec(); if the daemon later dies uncleanly, the pidfile keeps a stale pid. With -p given, do_procinit() checks only the pidfile pid and never scans /proc, so a stale pidfile silently breaks both start and stop (stop signals nothing, its rm -f never runs).

2. -x majestic (bare name) can only match by luck. pid_is_exec() compares the pattern against the full /proc/PID/exe symlink (e.g. /usr/bin/majestic) and then against argv[0] from /proc/PID/cmdline; a bare majestic never matches the exe link, so detection is fragile.

A liveness pre-check in the crontab (pgrep majestic) is extra state that can also go stale -- e.g. a lingering zombie whose comm is still majestic makes pgrep succeed while no usable daemon exists.

Fix

• Use the absolute path /usr/bin/majestic as DAEMON so busybox -x matches via /proc/PID/exe.
• Drop the pidfile entirely: start = -b -S -q -x "$DAEMON", stop = -K -q -x "$DAEMON". Without -p, start-stop-daemon scans /proc itself, so there is no stale state to poison it. Repeated start calls are idempotent: busybox refuses with a non-zero exit when the daemon is already running and does not touch the live instance.

This also makes the script safe for an unconditional crond watchdog (* * * * * /etc/init.d/S95majestic start): the daemon manager itself decides whether a copy is already running, with no pgrep/pidof pre-check to go stale.

Verification (on device, OpenIPC SSC335DE, busybox 1.36.1)

• Stock script + stock watchdog line: kill -9 -> majestic still dead at t+75 s and t+135 s while cron runs the line every minute; pidfile holds a wrong pid.
• Fixed script + unconditional cron line: kill -9 -> the next cron tick restarts majestic (RTSP :554 listening, stable across a 3-minute observation window); while healthy, the per-minute start is refused cleanly (FAIL (already running), rc=1) and the running daemon is untouched.
• Nine consecutive kill -9 -> restart cycles with the fixed invocation all succeeded; start, stop and restart actions verified individually.
• The production camera has been running the fixed script continuously since deployment.

Notes

• general/overlay/etc/init.d/S60crond uses the same -b -m -p pattern; it did not fail on the test hardware, so it is intentionally left untouched.
• On this SoC family a freshly killed majestic can refuse to re-initialize the sensor/VENC HAL for a couple of minutes (silent exit, no kernel messages). A per-minute watchdog absorbs this by simply retrying; a one-shot restart does not.

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

PR Summary by Qodo

Majestic: make S95majestic start/stop reliable without pidfile (watchdog-safe)

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Remove pidfile dependency so stale PIDs can’t block restart after crashes/OOM-kills.
• Match the daemon by absolute exec path to make busybox start-stop-daemon detection reliable.
• Make repeated start calls idempotent and safe for unconditional cron watchdogs.

Diagram

graph TD
  Cron["crond watchdog"] --> Init["S95majestic init"] --> SSD["start-stop-daemon"] --> Proc["/proc scan"] --> Maj["majestic daemon"]
  Init -->|"-x /usr/bin/majestic"| SSD

  subgraph Legend
    direction LR
    _job["Cron job"] ~~~ _file["Init script"] ~~~ _proc(["Process tool"]) ~~~ _k[("Kernel procfs")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep pidfile but validate against /proc

• ➕ Maintains traditional pidfile tooling/expectations
• ➕ Can preserve more specific status reporting (pidfile contents)
• ➖ More shell logic and edge cases (stale PID, PID reuse, zombies)
• ➖ Still depends on correct pidfile lifecycle under busybox double-fork behavior

2. Use pgrep/pidof pre-check in script (exec-path match)

• ➕ Simple and explicit control over ‘already running’ detection
• ➕ Can be tuned to match cmdline/exe path
• ➖ Adds another stateful detector that can also misreport (zombies/comm matches)
• ➖ Duplicates logic already present in start-stop-daemon and varies across environments

3. Switch to a supervisor (procd/runit/s6) for majestic

• ➕ Stronger guarantees for respawn/backoff and health checking
• ➕ Centralized service management instead of cron watchdogs
• ➖ Bigger architectural change; likely not feasible on minimal OpenIPC/busybox setups
• ➖ More moving parts and config migration risk

Recommendation: The PR’s approach (drop pidfile; match via absolute exec path; let start-stop-daemon scan /proc) is the best fit for busybox environments because it eliminates the primary failure mode (stale pidfile becoming the source of truth) while keeping the init script minimal and making repeated start calls safely idempotent—ideal for cron-based watchdog usage.

Files changed (1) +4 / -6

Bug fix (1) +4 / -6

S95majesticRemove pidfile usage and match majestic by absolute exec path +4/-6

Remove pidfile usage and match majestic by absolute exec path

• Switch DAEMON to '/usr/bin/majestic' so busybox start-stop-daemon '-x' matching is reliable. Remove '-p $PIDFILE' and pidfile deletion, forcing liveness checks to be based on '/proc' scanning and making repeated 'start' calls watchdog-safe with a clearer failure message.

general/package/majestic/files/S95majestic

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

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Reload signal no longer works ✓ Resolved 🐞 Bug ≡ Correctness

Description

DAEMON was changed to /usr/bin/majestic, but reload) still runs killall -1 "$DAEMON"; this
won’t match the running process name (majestic) and makes S95majestic reload ineffective.

Code

general/package/majestic/files/S95majestic[3]

+DAEMON="/usr/bin/majestic"

Evidence

The script now assigns DAEMON to an absolute path, but reload) still uses killall with
$DAEMON. Other init scripts in this repo use killall with a bare daemon name (no slashes),
indicating the intended contract is “process name”, not “path”.

general/package/majestic/files/S95majestic[3-4]
general/package/majestic/files/S95majestic[37-39]
general/package/mdnsd-openipc/files/S50mdnsd[8-10]
general/package/mdnsd-openipc/files/S50mdnsd[44-46]
general/overlay/usr/sbin/sysupgrade[344-351]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`reload)` currently signals via `killall -1 "$DAEMON"`, but `DAEMON` is now an absolute path (`/usr/bin/majestic`). `killall` is used elsewhere in-tree with a bare process name, so `reload` will not signal the running majestic process.
## Issue Context
The PR intentionally changed `DAEMON` to an absolute path to improve `start-stop-daemon -x` matching, but `reload` still assumes `DAEMON` is a process name.
## Fix Focus Areas
- general/package/majestic/files/S95majestic[3-4]
- general/package/majestic/files/S95majestic[37-39]
## Suggested fix
Keep `DAEMON` as the full path for `start-stop-daemon`, but compute/use a separate process name for `killall`/log output (e.g., `DAEMON_NAME="$(basename "$DAEMON")"`) and change `reload)` to `killall -1 "$DAEMON_NAME"` (or switch `reload` to a `start-stop-daemon`-based HUP if supported).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Misleading start failure message ✓ Resolved 🐞 Bug ◔ Observability

Description

start() prints FAIL (already running) for any non-zero start-stop-daemon exit code, which can
mask real startup failures (e.g., missing/unexecutable binary) and makes watchdog logs misleading.

Code

general/package/majestic/files/S95majestic[12]

+		echo "FAIL (already running)"

Evidence

The updated code prints the same "already running" message for all failures, and this script does
not perform a pre-check like some other init scripts do (e.g., telnet checks for binary presence),
so other failure modes are plausible and would be misreported.

general/package/majestic/files/S95majestic[6-13]
general/scripts/legacy/init.d/S50telnet[9-13]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`start()` unconditionally maps any non-zero `start-stop-daemon` status to the message `FAIL (already running)`. Non-zero can also occur for other reasons (e.g., exec failure), so the message is incorrect in those cases.
## Issue Context
This PR changes the message from a generic `FAIL` to a specific reason without checking the actual failure condition.
## Fix Focus Areas
- general/package/majestic/files/S95majestic[8-13]
## Suggested fix
Either revert to a generic `FAIL` message, or capture `$?` and print `FAIL (rc=$rc)`; only print `already running` if you can reliably distinguish that case (e.g., via documented exit codes or an explicit “is running” check).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[flyrouter]

An example script that handles the task if needed.

(while true; do sleep 15; checker=$(pidof majestic); if [[ -z $checker ]]; then /etc/init.d/S95majestic start; fi; done) &

[flyrouter]

I haven't approved the changes you sent over yet. Sorry.