majestic: make S95majestic stop work when the pidfile is stale

[phedoreanu]

majestic's built-in watchdog respawns the process outside start-stop-daemon's knowledge, so /var/run/majestic.pid goes stale. stop then kills nothing (start-stop-daemon -K targets the dead pid), and restart starts a second majestic next to the surviving one — two daemons fight over the encoder, and config edits appear to take effect while the running process never loaded them.

Observed on a Hi3518EV200 camera: the pidfile said 3408, majestic ran as pid 869; /etc/init.d/S95majestic restart exited 0 having restarted nothing, and /api/v1/config.json confirmed the old config was still live.

After the pidfile kill, this verifies the daemon is actually gone with pidof and falls back to killall (TERM, then KILL) — matching by name, the one identity that cannot go stale. The pidfile is always removed so the next start begins clean.

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

PR Summary by Qodo

Fix S95majestic stop/restart when /var/run/majestic.pid is stale

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Add stale-pidfile-safe stop logic for majestic when its watchdog respawns it
• Verify the daemon is actually gone via pidof, then fall back to killall TERM/KILL
• Always remove the pidfile so subsequent starts begin from a clean state

Diagram

graph TD
  A["S95majestic: stop()"] --> B["start-stop-daemon -K (pidfile)"] --> C{"pidof majestic?"}
  C -->|"no"| D["rm pidfile; report OK"]
  C -->|"yes"| E["killall TERM; then KILL"] --> D
  F["majestic daemon"] --> B

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use start-stop-daemon name-based killing (if supported)

• ➕ Avoids relying on a potentially stale pidfile
• ➕ Keeps all lifecycle handling within start-stop-daemon
• ➖ Not all start-stop-daemon builds expose consistent name/exe matching options
• ➖ May still risk matching the wrong process if naming/exe matching is weak

2. Replace killall with pgrep/pkill -x (exact match) fallback

• ➕ Exact-name matching can be safer than generic killall behavior
• ➕ More control over selecting which PIDs to terminate
• ➖ pgrep/pkill may not be available on minimal BusyBox images
• ➖ Adds extra conditional logic and tooling assumptions

Recommendation: The chosen approach (pidfile kill, verify with pidof, then killall fallback) is pragmatic for embedded/BusyBox environments where majestic’s watchdog can invalidate pidfiles. It minimizes behavioral change (still prefers pidfile semantics) while ensuring stop/restart correctness by falling back to the one stable identifier (process name) and always cleaning up the pidfile for a clean next start.

Files changed (1) +13 / -5

Bug fix (1) +13 / -5

S95majesticHarden stop() against stale majestic pidfile with pidof/killall fallback +13/-5

Harden stop() against stale majestic pidfile with pidof/killall fallback

• Adds an explanation comment describing how majestic’s watchdog can stale the pidfile and break stop/restart. Updates stop() to attempt pidfile-based termination, verify the daemon exited via pidof, fall back to killall TERM then KILL if needed, and always remove the pidfile before reporting success.

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. Stop reports success blindly ✓ Resolved 🐞 Bug ☼ Reliability

Description

S95majestic's stop() now unconditionally prints "OK" and removes the pidfile without checking
whether majestic is still running after start-stop-daemon/killall. This can make stop/restart claim
success while the daemon (or a respawned instance) is still alive, re-enabling duplicate-daemon
situations and making failures hard to diagnose.

Code

general/package/majestic/files/S95majestic[R32-33]

+	rm -f "$PIDFILE"
+	echo "OK"

Evidence

The updated stop() always deletes the pidfile and prints OK regardless of whether
start-stop-daemon/killall actually terminated the daemon, and it performs no final pidof
verification. Other init scripts in the repo retain a FAIL path based on start-stop-daemon’s return
code, highlighting that majestic is now masking failures.

general/package/majestic/files/S95majestic[23-34]
general/overlay/etc/init.d/S01syslogd[23-32]

Agent prompt

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

## Issue description
`stop()` always prints `OK` and deletes the pidfile even if majestic remains running after the kill attempts. This hides real shutdown failures and allows `restart` to proceed while a process is still alive.
## Issue Context
The PR adds a `pidof` + `killall` fallback to handle stale pidfiles. However, there is no final liveness check, and no failure reporting path.
## Fix Focus Areas
- general/package/majestic/files/S95majestic[23-34]
## Implementation notes
- Capture the exit status of the stop sequence.
- After the TERM + KILL attempts, re-check `pidof "$DAEMON"`.
- If still running, print `FAIL` (or similar) and return a non-zero status so `restart` can avoid starting a second instance.
- Only print `OK` once `pidof` no longer finds the daemon.
- Optionally: loop/wait with a short timeout rather than fixed sleeps, to reduce race windows.

ⓘ 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 ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗