majestic: start it again after a crash, and stop after five in ten minutes

[openipc-ai]

Symptom

Nothing restarts majestic when it dies. On every board, one crash leaves the camera without video until someone restarts majestic or power-cycles the camera.

Change

S95majestic now runs majestic under a small supervisor: the same script, called with supervise.

• Restarted: a majestic that dies of a fault (SIGSEGV, SIGBUS (7, or 10 on MIPS), SIGILL, SIGFPE, SIGABRT, or SIGKILL from the out-of-memory killer) is started again after 5 s, then 30 s, then 2 min.
• Crash loop: after five crashes within ten minutes it stops and leaves /etc/crash/majestic.loop (utc=, crashes=, signal=). The web interface reports it (crashlog: majestic's own crashes, on the page and sent with their build majestic-webui#648).
• Not restarted:
  • a majestic stopped on purpose (SIGTERM, SIGINT) or one that exits by itself;
  • one that crashes while /tmp/sysupgrade.lock exists, since the upgrade brings majestic back itself (and sysupgrade's SIGQUIT doesn't end majestic anyway).
• stop: ends the supervisor first, including while it is waiting to restart, so nothing starts majestic behind the stopper. The supervisor's pid is checked against its command line before it is signalled.
• start: clears the loop note, since it describes a run that is over.
• Unchanged: majestic is still found by executable (-x), keeps argv[0], -s and the inherited SIGHUP ignore. pidof majestic and S99bootok's health check see the same process as before. A majestic that keeps crashing at boot still fails bootok's window and escalates to failsafe, as before.

When majestic dies it now also leaves a crash dump in /etc/crash, which the WebUI can send (OpenIPC/website#415, OpenIPC/majestic-webui#648). The restart doesn't touch it.

Hardware evidence

gk7205v300 + imx335 (master+988f385, 2026-10-05), with this S95majestic installed in place of the shipped one and afterwards restored:

Starting majestic: OK
majestic=8109 supervisor=8107
 8107 root      0:00 sh /etc/init.d/S95majestic supervise
 8109 root      0:03 majestic -s
(kill -SEGV 8109)
after crash: majestic=
after 8s: majestic=8130
user.notice majestic: crashed (signal 11); starting it again in 5s

• Second crash within the window, then stop during its wait:

  user.notice majestic: crashed (signal 11); starting it again in 30s
  Stopping majestic: OK
  35s after stop: majestic= supervisor=
  

• SIGQUIT: kill -3 majestic (what sysupgrade sends) left the same pid running.
• restart: brought both back (majestic=8226 supervisor=8224).

T31 + sc2332 (master+3f53bec, 2026-09-26; MIPS, where SIGBUS is 10):

majestic=18872 supervisor=18870
(kill -BUS 18872)
after SIGBUS + 9s: majestic=18912
user.notice majestic: crashed (signal 10); starting it again in 5s

Both cameras were put back on their own S95majestic afterwards.

Tests

.github/scripts/test_majestic_supervise.sh runs the real script against a stub majestic and a stub start-stop-daemon, in both the shipped and the comment-stripped form. It checks:

• five starts and then the loop note;
• SIGABRT is handled like SIGSEGV;
• SIGTERM and a clean exit are not restarted;
• nothing restarts under the upgrade lock;
• start/stop, a second start, and stop during the wait;
• a reused pid is never killed.

It is wired into shell-tests.yml and passes locally.

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

PR Summary by Qodo

Restart majestic after crashes with bounded retries

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Supervise majestic so fault-related exits restart after increasing delays instead of leaving
 cameras without video.
• Stop after five crashes in ten minutes and record a loop note for the WebUI.
• Preserve intentional stops and upgrade ownership; test lifecycle behavior in CI.

Diagram

graph TD
  Super["S95 supervisor"] --> Maj["Majestic process"] --> Fault{"Fault signal?"} -->|yes| Upgrade{"Upgrade locked?"} -->|no| Limit{"Five in ten?"} -->|no| Backoff["Retry delay"] --> Maj
  Fault -->|no| End["Stop supervision"]
  Upgrade -->|yes| End
  Limit -->|yes| Note["Crash loop note"] --> End
  Super -->|stop requested| End

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use BusyBox init respawn

• ➕ Reuses the existing init supervisor instead of maintaining a separate process.
• ➖ Would require reworking the current init-script lifecycle and adding separate crash limits, signal filtering, and upgrade coordination.

Recommendation: Keep supervision in S95majestic. It preserves the existing start-stop-daemon and init-script contracts while supporting bounded, fault-specific retries; BusyBox init respawn alone does not provide those policies.

Files changed (3) +272 / -7

Bug fix (1) +93 / -7

S95majesticSupervise majestic with bounded crash restarts +93/-7

Supervise majestic with bounded crash restarts

• Starts majestic under a script-based supervisor that retries specified fault signals with increasing delays and records a loop note after five crashes within ten minutes. Suppresses retries during sysupgrade, preserves intentional-stop behavior, and stops the verified supervisor before stopping majestic.

general/package/majestic/files/S95majestic

Tests (1) +163 / -0

test_majestic_supervise.shAdd sandboxed supervisor regression tests +163/-0

Add sandboxed supervisor regression tests

• Runs the real init script with stubbed majestic and start-stop-daemon processes and shortened delays. Covers crash limits and notes, intentional exits, upgrade locks, start/stop behavior, stopping during backoff, and reused supervisor PIDs.

.github/scripts/test_majestic_supervise.sh

Other (1) +16 / -0

shell-tests.ymlRun majestic supervisor tests in CI +16/-0

Run majestic supervisor tests in CI

• Adds a shell-tests job for the new regression suite. Runs it against both the source init script and the comment-stripped form shipped on cameras.

.github/workflows/shell-tests.yml

[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. A quick stop can leave majestic running ✓ Resolved

Description

start() returns immediately after backgrounding the supervisor, before supervise() writes its
PID file or starts majestic. If stop runs in that interval, it sees neither process and returns
successfully, after which the supervisor can start majestic.

Code

general/package/majestic/files/S95majestic[R70-72]

+		exec sh "$0" supervise
+	) </dev/null >/dev/null 2>&1 &
+	echo "OK"

Evidence

The new background launch returns at lines 70–72, whereas the PID is not recorded until line 97.
Stop signals only a supervisor found through that file and determines completion solely by whether
majestic is currently running. The new test waits for a majestic start and then another 0.3 seconds
before calling stop, so it does not exercise this interval.

general/package/majestic/files/S95majestic[69-72]
general/package/majestic/files/S95majestic[93-101]
general/package/majestic/files/S95majestic[134-149]
.github/scripts/test_majestic_supervise.sh[120-130]

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` reports success before the supervisor is registered, allowing an immediate `stop` to miss it and return before majestic starts.
## Fix Focus Areas
- general/package/majestic/files/S95majestic[55-72]
- general/package/majestic/files/S95majestic[93-101]
- general/package/majestic/files/S95majestic[134-149]
## Recommended Fix
Coordinate start and stop through an atomic registration or shared lock. Do not let start report completion until stop can reliably find and terminate the launched supervisor; add a test that stops immediately after start.

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

---

2. An upgrade can get an unwanted restart ✓ Resolved

Description

supervise() checks the upgrade lock immediately after a crash but does not check it again when its
backoff sleep ends. If sysupgrade creates the lock during that sleep, the supervisor still attempts
to start majestic while the upgrade owns the camera.

Code

general/package/majestic/files/S95majestic[R105-106]

+		# An upgrade has the camera: it brings majestic back itself.
+		[ -f "$UPGRADE_LOCK" ] && break

Evidence

The lock is tested at line 106, followed by a sleep at lines 126–129 and an unconditional next
iteration into the launch at line 101. Sysupgrade creates /tmp/sysupgrade.lock when an upgrade
starts; the regression test covers only a lock that already exists before the crash.

general/package/majestic/files/S95majestic[100-106]
general/package/majestic/files/S95majestic[123-130]
general/overlay/usr/sbin/sysupgrade[2083-2112]
.github/scripts/test_majestic_supervise.sh[113-118]

Agent prompt

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

## Issue description
An upgrade that begins during a crash-restart delay is not covered by the supervisor's earlier lock check.
## Fix Focus Areas
- general/package/majestic/files/S95majestic[100-106]
- general/package/majestic/files/S95majestic[123-130]
- .github/scripts/test_majestic_supervise.sh[113-118]
## Recommended Fix
Check the upgrade lock again after the delay and before attempting the next majestic launch. Add a regression test that creates the lock during a restart delay.

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

---

3. Concurrent starts can escape a later stop ✓ Resolved

Description

start() checks for an existing process without coordinating that check with its background launch,
and each supervisor overwrites the same PID file. Two starts that pass the check together can leave
a live supervisor untracked; if it is waiting after a crash, stop cannot signal it and it can
restart majestic afterward.

Code

general/package/majestic/files/S95majestic[R57-60]

+	if running || supervising; then
+		echo "OK (already running)"
+		return 0
+	fi

Evidence

Both invocations can pass lines 57–60 before either background launch is visible. Each supervisor
writes the shared file at line 97 and removes it at line 131, while stop signals only the PID
currently read from that file. The test's second start is sequential, after it has observed the
first running.

general/package/majestic/files/S95majestic[55-72]
general/package/majestic/files/S95majestic[93-101]
general/package/majestic/files/S95majestic[126-138]
.github/scripts/test_majestic_supervise.sh[123-130]

Agent prompt

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

## Issue description
Concurrent starts can pass the same existence check and create supervisors that overwrite or remove one another's PID file, leaving a restart worker outside stop's control.
## Fix Focus Areas
- general/package/majestic/files/S95majestic[55-72]
- general/package/majestic/files/S95majestic[93-101]
- general/package/majestic/files/S95majestic[130-149]
## Recommended Fix
Serialize the check-and-launch operation, make PID-file ownership explicit, and remove the file only when it still belongs to the exiting supervisor. Test concurrent starts followed by stop during backoff.

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

---

4. A stale PID can stop another process ✓ Resolved

Description

supervising() accepts any process whose command line contains the substring  supervise, rather
than identifying this script and its supervise invocation. If the PID file is stale and its number
is reused by a process with that text in its arguments, stop() sends that unrelated process
SIGTERM.

Code

general/package/majestic/files/S95majestic[R78-79]

+	pid=$(cat "$SUPERVISOR" 2>/dev/null)
+	[ -n "$pid" ] && tr '\0' ' ' < "/proc/$pid/cmdline" 2>/dev/null | grep -q " supervise"

Evidence

The new PID check greps only for  supervise, and stop kills the PID it read when that check
succeeds. The stale-PID regression case uses sleep 999, which cannot match that substring and
therefore does not establish that a reused PID with matching arguments is safe.

general/package/majestic/files/S95majestic[75-80]
general/package/majestic/files/S95majestic[134-138]
.github/scripts/test_majestic_supervise.sh[153-160]

Agent prompt

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

## Issue description
A reused PID is accepted as the supervisor based solely on a generic command-line substring and can receive stop's SIGTERM.
## Fix Focus Areas
- general/package/majestic/files/S95majestic[75-80]
- general/package/majestic/files/S95majestic[134-138]
- .github/scripts/test_majestic_supervise.sh[153-160]
## Recommended Fix
Validate the expected script path and supervise argument together before signaling the PID. Extend the stale-PID test with an unrelated process whose arguments contain ` supervise`.

ⓘ 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 show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[openipc-ai]

Addressed Qodo's review in b5d2321:

1. Quick stop after start: start writes the supervisor's pid itself ($!, since the subshell execs into the supervisor) and returns only once that pid is the supervisor. A stop right after a start finds it. Test 10.
2. Upgrade during the wait: the upgrade lock is checked before every start of majestic, including the first and each one after a wait. Test: lock created during a 2 s backoff, no second start.
3. Concurrent starts: start and stop serialize on a lock directory holding the holder's pid. The lock is taken over only when that process is gone. Test 9; also on a gk7205v300, where two concurrent starts gave OK and OK (already running), one supervisor, and stop left nothing running.
4. Stale pid: the supervisor is identified by its last two arguments, this script's name and supervise. Test 8: a process whose last argument is supervise is not signalled.

Also classified the new test in ci-matrix.py; the selector failed on an unclassified file.