[floppyhammer]

Fix a deadlock where calling StopRxLoop() during device initialization (inside Init()) was ignored and overwritten.

Root cause:
StartRxLoop() unconditionally cleared the stop flag (should_stop / _rx_stop) upon entry. If StopRxLoop() was called while Init() was executing (e.g. during IQK calibration or channel setup), StartRxLoop() wiped out the pending stop request and launched bulk_read_async_loop indefinitely, deadlocking the caller waiting on the worker thread.

Changes:

• Clear stop flag at entry of Init() to mark the start of initialization.
• In StartRxLoop(), check whether stop was requested (should_stop / _rx_stop) before resetting the flag. Abort StartRxLoop() immediately if true.

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

PR Summary by Qodo

Prevent RX initialization from discarding stop requests

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Preserve stop requests made during device initialization across four radio families.
• Skip RX-loop startup when a device or global stop is pending, preventing an indefinite worker
 wait.

Diagram

graph TD
  Init["Init"] --> BringUp["Device bring-up"] --> Start["StartRxLoop"] --> Check{"Stop requested?"}
  Stop["StopRxLoop"] --> Flag["Stop flag"] --> Check
  Init -->|clear| Flag
  Check -->|yes| Abort["Return early"]
  Check -->|no| Read["Async bulk read"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Serialize RX lifecycle transitions

• ➕ Could define reliable behavior for concurrent Init, StartRxLoop, and StopRxLoop calls, including a stop arriving between the check and flag reset.
• ➖ Requires broader locking or state-machine changes across four device implementations and their blocking worker loops.

Recommendation: The targeted change fits the reported stop-during-bring-up window and preserves the existing restart behavior. If concurrent StartRxLoop and StopRxLoop calls must also be guaranteed, use a synchronized lifecycle design: the current check followed by a reset does not make that transition atomic.

Files changed (4) +12 / -0

Bug fix (4) +12 / -0

RtlJaguarDevice.cppPreserve Jaguar1 stop requests through initialization +3/-0

Preserve Jaguar1 stop requests through initialization

• Clears should_stop before monitor-mode setup. StartRxLoop returns before RX setup when should_stop or the global stop flag is set.

src/jaguar1/RtlJaguarDevice.cpp

RtlJaguar2Device.cppHonor Jaguar2 stops before RX worker startup +3/-0

Honor Jaguar2 stops before RX worker startup

• Clears _rx_stop before bring-up and checks device and global stop flags before starting RX-related work.

src/jaguar2/RtlJaguar2Device.cpp

RtlJaguar3Device.cppHonor Jaguar3 stops before RX handoff +3/-0

Honor Jaguar3 stops before RX handoff

• Clears _rx_stop before channel setup and calibration. StartRxLoop returns on a pending device or global stop before taking over bulk-IN.

src/jaguar3/RtlJaguar3Device.cpp

RtlKestrelDevice.cppPreserve Kestrel stops during monitor bring-up +3/-0

Preserve Kestrel stops during monitor bring-up

• Clears _rx_stop at Init entry and checks device and global stop flags before starting the RX loop, after stopping the TX WP-release drain.

src/kestrel/RtlKestrelDevice.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. Stopped receivers cannot restart ✓ Resolved

Description

In all four backends, StartRxLoop() now returns on the stop flag before reaching the existing code
that clears a request left by StopRxLoop(). After a caller stops and joins a receive loop, another
StartRxLoop() without Init() immediately returns instead of receiving; on Kestrel it first stops
the USB transmit-release drain, leaving neither reader running.

Code

src/kestrel/RtlKestrelDevice.cpp[R444-445]

+  if (_rx_stop || g_devourer_should_stop)
+    return;

Evidence

The interface expressly promises restart after the blocking loop returns. Each backend's StopRxLoop
only sets its flag, while the added guard now tests that flag before the former restart reset.
Kestrel additionally calls stop_wp_drain before the guard, and that drain consumes transmit-release
reports.

src/IRadio.h[59-67]
src/jaguar1/RtlJaguarDevice.h[147-147]
src/jaguar1/RtlJaguarDevice.cpp[1806-1812]
src/jaguar2/RtlJaguar2Device.h[60-60]
src/jaguar2/RtlJaguar2Device.cpp[597-603]
src/jaguar3/RtlJaguar3Device.h[57-57]
src/jaguar3/RtlJaguar3Device.cpp[212-218]
src/kestrel/RtlKestrelDevice.h[66-66]
src/kestrel/RtlKestrelDevice.cpp[123-137]
src/kestrel/RtlKestrelDevice.cpp[440-446]

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 stop requested during Init must prevent its receive loop from starting, but a stop left by a completed receive loop must not prevent a later standalone StartRxLoop. The new guards treat both cases alike; Kestrel also stops its transmit-release drain before returning.
## Fix Focus Areas
- src/jaguar1/RtlJaguarDevice.cpp[1806-1812]
- src/jaguar2/RtlJaguar2Device.cpp[597-603]
- src/jaguar3/RtlJaguar3Device.cpp[212-218]
- src/kestrel/RtlKestrelDevice.cpp[440-446]
## Recommended Fix
Track whether a stop request belongs to an initialization currently in progress, using synchronization appropriate for calls from another thread. Abort that initialization's receive start when requested, but clear a completed loop's stale stop request for a standalone restart. In Kestrel, do not relinquish the transmit-release drain when the receive start is aborted.

ⓘ 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 ↗

[josephnef]

Reviewed with a hardware reproducer (Init on a thread, StopRxLoop() at N ms, join with a 20 s timeout), both libraries built from clean worktrees — master f46c28a and this PR's head — run as root with no kernel driver bound.

Build	Adapter	Stop at 100 / 400 / 1000 / 2500 ms	Stop+join, then standalone StartRxLoop	StopRxLoop before Init
master	RTL8822BU (Jaguar2, ch6)	DEADLOCK / DEADLOCK / DEADLOCK / ok	ok	ok
master	RTL8812EU (Jaguar3, ch36)	DEADLOCK / DEADLOCK / ok / ok	ok	ok
PR	RTL8822BU	ok / ok / ok / ok	ok, frames flow	ok, frames flow
PR	RTL8812EU	ok / ok / ok / ok	ok, frames flow	ok, frames flow

DEADLOCK = the Init thread never returned (frames kept arriving the whole 20 s). "ok" on the PR = Init returned 0.5–1.4 s after the stop, i.e. bring-up completed and StartRxLoop refused to start. The last two columns are the scenarios the Qodo thread raised; both behave correctly. Jaguar1 and Kestrel were not on the bench, so those two are compile-verified only.

The fix is real and the mechanism works. Before merge I'd like these addressed, ranked:

1. Coverage. Rtl8733bDevice::StartRxLoop and Mt7612uRadio::StartRxLoop have the same unconditional _rx_stop = false and both Inits call StartRxLoop the same way, so the identical deadlock remains on those two backends. The title says the library; it currently fixes four of six.
2. The InitWrite + StartRxLoop-on-a-thread shape (DEVOURER_TX_WITH_RX=thread) keeps the same race: a stop issued before that thread reaches StartRxLoop takes the "stale, clear it" branch. Fine to leave out of scope, but say so in the IRadio.h StopRxLoop comment so callers know which window is covered.
3. Simpler mechanism (inline comments): a private start_rx_loop(proc, bool clear_stale) that Init calls with false and the public override with true needs no atomic, no scope guard, and has no window at all.
4. The clear in Init should be its first statement (inline comment).
5. Indentation: 4-space blocks in 2-space files, and the Jaguar3 StartRxLoop prologue re-indented, which churns untouched comment lines.
6. The red qodo-gate is only because resolving the thread doesn't re-trigger it — a reply inside that thread re-runs the check.

[josephnef]

Re-review of 6e36580. Every point from the previous round is addressed: the _init_in_progress atomic and the pasted ScopeGuard are gone in favour of the private start_rx_loop(proc, clear_stale), the clear is now the first statement of every Init, the Kestrel early return moved ahead of stop_wp_drain(), the indentation matches, and the RTL8733B and MT7612U backends get the same guard. Two nits inline, neither blocking.

Hardware re-run of yesterday's reproducer at this head (Init on a thread, StopRxLoop at N ms, 20 s join timeout), three backends plugged today, no kernel driver bound:

backend	stop at 100 / 400 / 1000 / 2500 ms	restart after stop+join	stale stop before Init
8822BU (Jaguar2, ch6)	OK, Init returns 1420 / 1097 / 473 / 10 ms later	OK, 314 frames	OK, 268 frames
8812EU (Jaguar3, ch36)	OK, 845 / 563 / 100 / 60 ms	OK, 3778 frames	OK, 5244 frames
8832CU (Kestrel, ch36)	OK, 3376 / 3058 / 2385 / 1012 ms	OK, 2497 frames	OK, 1183 frames

Kestrel was unmeasured in the previous round; its bring-up is the longest (~3.4 s), so every stop up to 2500 ms lands inside it and all four are honoured. Jaguar1, RTL8733B and MT7612U are not on the bench today and are reviewed by reading only.

🤖 Generated with Claude Code