[josephnef]

Follow-ups from #504 (left open by #503).

• src/IRadio.h: the StopRxLoop note stated a race "window" and what was out of scope; it now states the contract. A stop issued while Init() is still bringing the chip up ends Init without a loop. A stop issued before Init() or before a standalone StartRxLoop() is stale and cleared by them, which includes a stop that lands between InitWrite() and a StartRxLoop() on another thread.
• The signal-flag check at the entry of every start_rx_loop gets its reason: it skips the bring-up-side setup for a loop whose own predicate would exit on its first pass.
• The reproducer that validated fix(devourer): prevent deadlock when StopRxLoop is called during Init #503 moves into the tree: tests/stop_during_init_probe.cpp (target StopDuringInitProbe, --no-reset for Kestrel) and tests/stop_during_init.sh (VID/PID/CHANNEL/NO_RESET, one verdict line, exit code), plus a tests/README.md entry.

Validation through the promoted script at this commit, no kernel driver bound:

backend	Init returns after a stop at 100 / 400 / 1000 / 2500 ms	restart after stop	stale stop before Init
8822BU (Jaguar2, ch6)	1409 / 1097 / 474 / 100 ms	OK, 216 frames	OK, 318 frames
8812EU (Jaguar3, ch36)	866 / 543 / 80 / 40 ms	OK, 4570 frames	OK, 4510 frames
8832CU (Kestrel, ch36)	3346 / 3114 / 2425 / 999 ms	OK, 3373 frames	OK, 1745 frames

The 8812AU, RTL8733B and MT7612U carry the same guard and were not on the bench; that item stays open on #504.

🤖 Generated with Claude Code

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

PR Summary by Qodo

Document StopRxLoop semantics and add an Init-stop bench probe

🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Clarify when StopRxLoop must end Init and when earlier stops are stale.
• Promote hardware regression checks for stops during Init, standalone restarts, and stale stops.
• Document the existing early-exit guard across six radio backends.

Diagram

graph TD
  S["Scenario script"] --> P["USB probe"] --> R["IRadio backends"] --> I{"Scenario"} -->|"stop during Init"| J["Bounded join"] --> V["Verdict"]
  I -->|"stale or restart"| F["RX frame check"] --> V

Loading

High-Level Assessment

Keep the hardware probe as a bench test rather than treating a mock or ordinary unit test as equivalent: the regression depends on real adapter bring-up timing. The PR appropriately documents the existing guards without presenting them as a new behavioral fix.

Files changed (11) +289 / -3

Documentation (8) +41 / -3

IRadio.hSpecify StopRxLoop timing and stale-stop behavior +6/-3

Specify StopRxLoop timing and stale-stop behavior

• Replaces the race-window note with the contract for stops during Init and stale stops before Init or standalone StartRxLoop, including the InitWrite-to-threaded-StartRxLoop case.

src/IRadio.h

RtlJaguarDevice.cppExplain Jaguar1’s pre-setup signal guard +2/-0

Explain Jaguar1’s pre-setup signal guard

• Documents why an existing global-stop check returns before RX setup.

src/jaguar1/RtlJaguarDevice.cpp

RtlJaguar2Device.cppExplain Jaguar2’s pre-setup signal guard +2/-0

Explain Jaguar2’s pre-setup signal guard

• Documents why an existing global-stop check returns before RX setup.

src/jaguar2/RtlJaguar2Device.cpp

RtlJaguar3Device.cppExplain Jaguar3’s pre-setup signal guard +2/-0

Explain Jaguar3’s pre-setup signal guard

• Documents why an existing global-stop check returns before RX setup.

src/jaguar3/RtlJaguar3Device.cpp

RtlKestrelDevice.cppExplain Kestrel’s pre-setup signal guard +2/-0

Explain Kestrel’s pre-setup signal guard

• Documents why an existing global-stop check returns before RX setup.

src/kestrel/RtlKestrelDevice.cpp

Mt7612uRadio.cppExplain MT7612U’s pre-setup signal guard +2/-0

Explain MT7612U’s pre-setup signal guard

• Documents why an existing global-stop check returns before RX setup.

src/mt7612u/Mt7612uRadio.cpp

Rtl8733bDevice.cppExplain RTL8733B’s pre-setup signal guard +2/-0

Explain RTL8733B’s pre-setup signal guard

• Documents why an existing global-stop check returns before RX setup.

src/rtl8733b/Rtl8733bDevice.cpp

README.mdDocument the stop-during-Init bench procedure +23/-0

Document the stop-during-Init bench procedure

• Explains the regression, invocation for three adapter families, DEADLOCK reporting, Kestrel’s no-reset requirement, and untested backends.

tests/README.md

Other (3) +248 / -0

CMakeLists.txtBuild the StopDuringInitProbe bench executable +10/-0

Build the StopDuringInitProbe bench executable

• Adds a hardware probe target linked against devourer and libusb. The accompanying script runs its scenario matrix; it is not registered as an automated CTest.

CMakeLists.txt

stop_during_init.shRun and score the six-scenario hardware matrix +54/-0

Run and score the six-scenario hardware matrix

• Runs four timed Init stops plus restart and stale-stop scenarios with configurable USB IDs, channel, timeout, and reset behavior. Prints per-scenario results and an overall exit-code verdict.

tests/stop_during_init.sh

stop_during_init_probe.cppProbe threaded Init stops and RX restartability +184/-0

Probe threaded Init stops and RX restartability

• Opens a USB radio and exercises timed stops, a standalone restart, and a pre-Init stale stop. Uses bounded joins to report hangs and frame counts to check that expected RX loops actually run.

tests/stop_during_init_probe.cpp

[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. Local users can truncate root-owned files ✓ Resolved

Description

stop_during_init.sh creates a predictable directory under /tmp and redirects stderr to
predictable filenames without checking whether those paths are symlinks. Before a documented sudo
run, a local user can pre-create the directory and a scenario’s .err symlink, causing root’s
redirection to truncate its target.

Code

tests/stop_during_init.sh[R44-45]

+  out=$(timeout $((TIMEOUT_S * 3)) "$PROBE" --vid "$VID" --pid "$PID" --channel "$CHANNEL" \
+          --timeout-s "$TIMEOUT_S" "${extra[@]}" "$@" 2>"$LOG/$name.err" | grep RESULT)

Evidence

The README recommends sudo; the script uses a fixed /tmp path, accepts an existing directory with
mkdir -p, and opens fixed scenario filenames through shell redirection. Nothing in that path
verifies ownership or rejects symlinks.

tests/README.md[581-585]
tests/stop_during_init.sh[30-45]
tests/stop_during_init.sh[50-52]

Agent prompt

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

## Issue description
The root-run script follows attacker-created log symlinks beneath its predictable default `/tmp` directory, allowing arbitrary file truncation.
## Fix Focus Areas
- tests/stop_during_init.sh[31-45]
## Recommended Fix
Create an exclusive, private log directory with `mktemp -d` for each run rather than trusting a pre-existing predictable directory. Keep its location available in failure output, and ensure any configurable log destination cannot reintroduce symlink-following writes.

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

---

2. The test guide repeats the stop contract ✓ Resolved

Description

The new tests/README.md section restates the StopRxLoop() lifecycle behavior already documented
in src/IRadio.h, including stale stops and the timing scenarios. This duplicates the header
contract in another guide and makes future changes harder to keep consistent across the two
descriptions.

Code

tests/README.md[R571-576]

+### `stop_during_init.sh`: a stop that lands inside `Init()`
+
+`StopRxLoop()` is the only way out of a blocking `Init()`, and a stop that
+lands while the chip is still being brought up used to be erased by the loop
+start that followed, leaving the caller joining a thread that never returns.
+The script drives `build/StopDuringInitProbe` through the contract on

Evidence

The changed README section repeats the contract that the checklist requires to live in the header,
describing stops during Init(), stale stops before Init(), and the probe scenarios instead of
only linking to the existing header documentation.

CLAUDE.md: Keep Documentation Scoped and Avoid Duplicating Header Contracts: CLAUDE.md: Keep Documentation Scoped and Avoid Duplicating Header Contracts: CLAUDE.md: Keep Documentation Scoped and Avoid Duplicating Header Contracts: CLAUDE.md: Keep Documentation Scoped and Avoid Duplicating Header Contracts
tests/README.md[571-579]

Agent prompt

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

## Issue description
The new test documentation duplicates the `StopRxLoop()` lifecycle contract already documented in `src/IRadio.h`, creating two descriptions that can drift apart.
## Fix Focus Areas
- tests/README.md[571-579]
## Recommended Fix
Keep the test-specific commands, scenarios, and hardware notes in `tests/README.md`, but replace the repeated contract explanation with a concise reference to `src/IRadio.h`.

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

---

3. Failed chip setup passes the stop test ✓ Resolved

Description

initstop prints outcome=OK after any normal return from Init(), without establishing that
initialization succeeded or that the stop reached an active initialization. On Kestrel, Init()
also returns normally when bring-up fails before starting RX, so a hardware or firmware failure can
produce a passing verdict without exercising the contract.

Code

tests/stop_during_init_probe.cpp[R180-183]

+  std::printf("RESULT mode=initstop outcome=OK stop_at_ms=%d init_return_ms=%ld "
+              "frames=%ld\n",
+              stop_at_ms, init_return_ms, frames.load());
+  return 0;

Evidence

The probe checks only that its Init thread returned, then emits OK unconditionally. Kestrel returns
from Init when BringUpMonitor fails, before calling its receive loop.

tests/stop_during_init_probe.cpp[138-151]
tests/stop_during_init_probe.cpp[180-183]
src/kestrel/RtlKestrelDevice.cpp[296-304]
src/kestrel/RtlKestrelDevice.cpp[169-180]

Agent prompt

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

## Issue description
The init-stop scenario reports success for Kestrel bring-up failures that return without ever starting RX.
## Fix Focus Areas
- tests/stop_during_init_probe.cpp[138-151]
- tests/stop_during_init_probe.cpp[180-183]
- src/kestrel/RtlKestrelDevice.cpp[296-304]
## Recommended Fix
Distinguish successful bring-up followed by an honored stop from an early initialization failure, using an explicit status or a verified working RX phase. Reject a scenario if initialization has already returned before its scheduled stop.

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

---

4. Alternate radios use the wrong settings ✓ Resolved

Description

The README places VID, PID, CHANNEL, and NO_RESET assignments before sudo, while the
script obtains them only from its environment. When sudo scrubs that environment, the
alternate-radio examples run with the 8822BU defaults; the Kestrel example also loses its required
no-reset setting.

Code

tests/README.md[R583-584]

+PID=0xa81a CHANNEL=36 sudo tests/stop_during_init.sh            # 8812EU
+VID=0x35bc PID=0x0101 CHANNEL=36 NO_RESET=1 sudo tests/stop_during_init.sh

Evidence

The new examples set variables in sudo's calling environment, but the script falls back to different
defaults if they are absent. This repository's test documentation explicitly notes that sudo scrubs
environment variables.

tests/README.md[155-162]
tests/README.md[581-585]
tests/stop_during_init.sh[26-29]

Agent prompt

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

## Issue description
Documented commands lose device selections when sudo resets the caller's environment.
## Fix Focus Areas
- tests/README.md[581-585]
- tests/stop_during_init.sh[16-18]
## Recommended Fix
Put assignments after `sudo`, for example `sudo env PID=0xa81a CHANNEL=36 tests/stop_during_init.sh`, and do the same for the Kestrel command and examples in the script.

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

---

View review recommended (1)

5. Stopped scripts can leave probes running ✓ Resolved

Description

cleanup calls pkill -x StopDuringInitProbe, although that executable name exceeds Linux’s
15-character process-name limit used for name matching. If the script is terminated while its child
is still active, the trap therefore cannot find that child and may leave it driving the adapter.

Code

tests/stop_during_init.sh[R35-36]

+cleanup() { pkill -x StopDuringInitProbe 2>/dev/null || true; }
+trap cleanup EXIT INT TERM

Evidence

The script names the executable StopDuringInitProbe but uses that full name with pkill -x in its
termination trap. Its timeout invocation does not record a child PID for cleanup; another bench
script documents and uses PID-based cleanup to avoid name-based targeting.

tests/stop_during_init.sh[24-25]
tests/stop_during_init.sh[35-45]
tests/mt7612u_ap_onair.sh[60-65]

Agent prompt

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

## Issue description
Name-based cleanup cannot reliably match the long probe executable name and does not identify this script's own child.
## Fix Focus Areas
- tests/stop_during_init.sh[35-45]
## Recommended Fix
Track the PID or process group launched for each scenario and terminate only that child from the trap. Reap it where possible instead of matching all processes by executable name.

ⓘ 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 add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗