diff --git a/CMakeLists.txt b/CMakeLists.txt index 9bcbab81..601729fe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -894,6 +894,16 @@ add_executable(BusyWindowProbe target_include_directories(BusyWindowProbe PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/examples/common) target_link_libraries(BusyWindowProbe PUBLIC devourer PRIVATE PkgConfig::libusb) +# Bench driver for the StopRxLoop contract on IRadio.h: a stop issued while +# Init() is still bringing the chip up must end Init without a loop, and a +# stale stop must not refuse the next loop. tests/stop_during_init.sh runs the +# matrix; a hung Init is reported as DEADLOCK, never waited out. +add_executable(StopDuringInitProbe + tests/stop_during_init_probe.cpp +) +target_include_directories(StopDuringInitProbe PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/examples/common) +target_link_libraries(StopDuringInitProbe PUBLIC devourer PRIVATE PkgConfig::libusb) + # Headless regression guard for the binary-stdin framing shared by the two # stream demos above (examples/common/stream_stdin.h). No libusb, no radio — just the # set_stdin_binary() + read_exact() path, so a text-mode regression (e.g. the diff --git a/src/IRadio.h b/src/IRadio.h index 9f8de0bf..299bede7 100644 --- a/src/IRadio.h +++ b/src/IRadio.h @@ -91,9 +91,12 @@ class IRadio { /* Ask a running StartRxLoop to exit (sets a flag; the caller then joins * whatever thread runs StartRxLoop). Default no-op. - * Note: Covers the Init + StartRxLoop race window; the InitWrite + - * StartRxLoop-on-a-thread shape (DEVOURER_TX_WITH_RX=thread) operates outside - * this window and is out of scope. */ + * + * A stop issued while Init() is still bringing the chip up is honoured: + * Init returns without starting its loop. A stop issued before Init() or + * before a standalone StartRxLoop() is stale and cleared by them (both are + * restartable), which includes a stop that lands between InitWrite() and a + * StartRxLoop() on another thread (DEVOURER_TX_WITH_RX=thread). */ virtual void StopRxLoop() {} virtual void SetMonitorChannel(SelectedChannel channel) = 0; diff --git a/src/jaguar1/RtlJaguarDevice.cpp b/src/jaguar1/RtlJaguarDevice.cpp index f31107db..135a1045 100644 --- a/src/jaguar1/RtlJaguarDevice.cpp +++ b/src/jaguar1/RtlJaguarDevice.cpp @@ -1809,6 +1809,8 @@ void RtlJaguarDevice::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { } void RtlJaguarDevice::start_rx_loop(Action_ParsedRadioPacket packetProcessor, bool clear_stale) { + /* A signal has already landed: skip the setup below rather than build it + * for a loop whose own predicate would exit on its first pass. */ if (g_devourer_should_stop) return; diff --git a/src/jaguar2/RtlJaguar2Device.cpp b/src/jaguar2/RtlJaguar2Device.cpp index 99222cf7..da3b6695 100644 --- a/src/jaguar2/RtlJaguar2Device.cpp +++ b/src/jaguar2/RtlJaguar2Device.cpp @@ -600,6 +600,8 @@ void RtlJaguar2Device::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { } void RtlJaguar2Device::start_rx_loop(Action_ParsedRadioPacket packetProcessor, bool clear_stale) { + /* A signal has already landed: skip the setup below rather than build it + * for a loop whose own predicate would exit on its first pass. */ if (g_devourer_should_stop) return; diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 56995c86..e1e782e7 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -215,6 +215,8 @@ void RtlJaguar3Device::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { } void RtlJaguar3Device::start_rx_loop(Action_ParsedRadioPacket packetProcessor, bool clear_stale) { + /* A signal has already landed: skip the setup below rather than build it + * for a loop whose own predicate would exit on its first pass. */ if (g_devourer_should_stop) return; diff --git a/src/kestrel/RtlKestrelDevice.cpp b/src/kestrel/RtlKestrelDevice.cpp index 226a75dd..17dce5a9 100644 --- a/src/kestrel/RtlKestrelDevice.cpp +++ b/src/kestrel/RtlKestrelDevice.cpp @@ -442,6 +442,8 @@ void RtlKestrelDevice::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { } void RtlKestrelDevice::start_rx_loop(Action_ParsedRadioPacket packetProcessor, bool clear_stale) { + /* A signal has already landed: skip the setup below rather than build it + * for a loop whose own predicate would exit on its first pass. */ if (g_devourer_should_stop) return; diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index 59597ea4..26430cd7 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -397,6 +397,8 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { } void Mt7612uRadio::start_rx_loop(Action_ParsedRadioPacket packetProcessor, bool clear_stale) { + /* A signal has already landed: skip the setup below rather than build it + * for a loop whose own predicate would exit on its first pass. */ if (g_devourer_should_stop) return; diff --git a/src/rtl8733b/Rtl8733bDevice.cpp b/src/rtl8733b/Rtl8733bDevice.cpp index 44ea3110..e59d1144 100644 --- a/src/rtl8733b/Rtl8733bDevice.cpp +++ b/src/rtl8733b/Rtl8733bDevice.cpp @@ -361,6 +361,8 @@ void Rtl8733bDevice::StartRxLoop( void Rtl8733bDevice::start_rx_loop( Action_ParsedRadioPacket packetProcessor, bool clear_stale) { + /* A signal has already landed: skip the setup below rather than build it + * for a loop whose own predicate would exit on its first pass. */ if (g_devourer_should_stop) return; diff --git a/tests/README.md b/tests/README.md index a9b84bc3..52be2f99 100644 --- a/tests/README.md +++ b/tests/README.md @@ -568,6 +568,29 @@ quiesce must still return, and a `tx.quiesce_timeout` event in the log says it never drained. `BUILD=` points at another tree, which is how a fix is shown to fix something rather than asserted to. +### `stop_during_init.sh`: a stop that lands inside `Init()` + +Drives `build/StopDuringInitProbe` through the `StopRxLoop` contract stated +on its declaration in `src/IRadio.h`: a stop at 100 / 400 / 1000 / 2500 ms +into `Init` (each must find Init still running, end it, and leave a chip +that then receives on a standalone loop), a standalone restart after a stop, +and a stale stop before `Init`. The frame checks need a channel with +traffic; ambient beacons do. + +```bash +sudo tests/stop_during_init.sh # 8822BU, ch6 +sudo env PID=0xa81a CHANNEL=36 tests/stop_during_init.sh # 8812EU +sudo env VID=0x35bc PID=0x0101 CHANNEL=36 NO_RESET=1 tests/stop_during_init.sh +``` + +`sudo env VAR=...`, not `VAR=... sudo`: sudo scrubs the environment the +script reads its knobs from. A `DEADLOCK` line means `Init` never came back; +the probe drops the USB lock and exits, so the adapter may need a re-plug. +The Kestrel run needs `NO_RESET=1`: a USB reset on its running firmware +re-enumerates the adapter out from under the probe. Measured on the 8822BU, +8812EU and 8832CU; the 8812AU, RTL8733B and MT7612U carry the same guard and +have not been run. + ### `kestrel_prich_onair.sh`: Kestrel per-channel BB programming Counts delivered frames across the configurations that the vendored diff --git a/tests/stop_during_init.sh b/tests/stop_during_init.sh new file mode 100755 index 00000000..a38dc70f --- /dev/null +++ b/tests/stop_during_init.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash +# Does a StopRxLoop() issued while Init() is still bringing the chip up end +# Init, and does a stale stop leave the next loop alone? Six scenarios on one +# adapter through build/StopDuringInitProbe (the contract is on StopRxLoop in +# src/IRadio.h): +# +# initstop at 100 / 400 / 1000 / 2500 ms -- the first two or three land +# inside bring-up on every generation (Kestrel's is ~3.4 s, so +# all four do there); the rest hit a running loop. Init must +# still be running when the stop lands, must return after it, +# and the chip it left behind must then receive on a standalone +# loop -- so a bring-up that failed and returned on its own +# cannot pass as an honoured stop. +# restart stop + join, then a standalone StartRxLoop on a new thread -- +# must run and see frames. +# stale StopRxLoop before Init -- Init's own loop must still run. +# +# The frame checks need a channel that carries traffic (ambient beacons do). +# +# sudo tests/stop_during_init.sh # 8822BU, ch6 +# sudo env PID=0xa81a CHANNEL=36 tests/stop_during_init.sh # 8812EU +# sudo env VID=0x35bc PID=0x0101 CHANNEL=36 NO_RESET=1 tests/stop_during_init.sh +# +# (`sudo env ...` rather than `VAR=... sudo`: sudo scrubs the caller's +# environment, and the script reads its knobs from its own.) +# +# A DEADLOCK line means the Init thread never came back: the probe drops the +# USB lock and _Exits, so the adapter may need a re-plug before the next run. +# Exit 0 = every scenario OK, 1 = at least one failed or produced no result. +set -u +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +PROBE="$ROOT/build/StopDuringInitProbe" +VID="${VID:-0x0bda}" +PID="${PID:-0xb812}" +CHANNEL="${CHANNEL:-6}" +NO_RESET="${NO_RESET:-0}" +TIMEOUT_S="${TIMEOUT_S:-20}" + +[ -x "$PROBE" ] || { echo "missing $PROBE (cmake --build build --target StopDuringInitProbe)" >&2; exit 1; } +# A fresh private directory per run: the script is documented as a sudo run, +# and a predictable path under /tmp would let any local user plant a symlink +# for root's stderr redirection to follow. +LOG="$(mktemp -d "${TMPDIR:-/tmp}/devourer-stop-during-init.XXXXXX")" +child="" +cleanup() { + # The probe's comm is truncated to 15 characters, so a name match cannot + # find it; kill the child this script started, by pid. + [ -n "$child" ] && kill "$child" 2>/dev/null + [ -n "$child" ] && wait "$child" 2>/dev/null + : +} +trap cleanup EXIT INT TERM + +extra=() +[ "$NO_RESET" = 1 ] && extra+=(--no-reset) +fail=0 +run() { # name args... + local name=$1; shift + local out + timeout $((TIMEOUT_S * 3)) "$PROBE" --vid "$VID" --pid "$PID" --channel "$CHANNEL" \ + --timeout-s "$TIMEOUT_S" "${extra[@]}" "$@" >"$LOG/$name.out" 2>"$LOG/$name.err" & + child=$! + wait "$child" + child="" + out=$(grep RESULT "$LOG/$name.out") + echo "[$VID:$PID ch$CHANNEL] $name: ${out:-NO-RESULT}" + case "$out" in *outcome=OK*) ;; *) fail=1 ;; esac + sleep 1 +} +for ms in 100 400 1000 2500; do run "initstop_$ms" --mode initstop --stop-at-ms "$ms"; done +run restart --mode restart --stop-at-ms 6000 --rx-ms 4000 +run stale --mode stale --rx-ms 6000 +if [ $fail = 0 ]; then echo "VERDICT: PASS"; rm -rf "$LOG"; else echo "VERDICT: FAIL (logs in $LOG)"; fi +exit $fail diff --git a/tests/stop_during_init_probe.cpp b/tests/stop_during_init_probe.cpp new file mode 100644 index 00000000..d99e2266 --- /dev/null +++ b/tests/stop_during_init_probe.cpp @@ -0,0 +1,213 @@ +/* stop_during_init_probe — exercises the StopRxLoop contract on IRadio.h + * (tests/stop_during_init.sh runs the matrix): + * + * initstop Init() on a thread; StopRxLoop() after --stop-at-ms; the Init + * thread must still be inside Init at the stop and must return + * within --timeout-s after it. Then a standalone StartRxLoop() + * runs for --rx-ms and must see frames: the chip the stop + * interrupted has to be a working receiver, so a bring-up that + * failed and returned on its own cannot pass as an honoured + * stop. (Before the fix StartRxLoop cleared the flag and the + * loop ran forever.) + * restart Init() on a thread; after --stop-at-ms StopRxLoop(); join; + * then StartRxLoop() on a new thread, run --rx-ms, StopRxLoop(), + * join. Both joins must return and the second loop must see + * frames: a stop left by a finished loop must not refuse a + * standalone restart. + * stale StopRxLoop() BEFORE Init(); Init() on a thread; after --rx-ms + * StopRxLoop(); join. RX must have run (frames > 0 on a channel + * that carries traffic): a pre-Init stale stop must not refuse + * Init's own loop. + * + * --no-reset skips libusb_reset_device before the claim (Kestrel: a reset + * on running firmware re-enumerates the adapter, src/kestrel/CLAUDE.md). + * Prints one line: RESULT mode= outcome= ... + * Exit 0 = OK, 1 = the contract failed, 3 = no adapter. + */ +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "DeviceSession.h" +#include "IRadio.h" +#include "UsbOpen.h" +#include "WiFiDriver.h" +#include "logger.h" + +using clk = std::chrono::steady_clock; + +static bool join_within(std::thread &t, std::atomic &done, int timeout_s, + long *elapsed_ms) { + const auto t0 = clk::now(); + while (!done.load() && + clk::now() - t0 < std::chrono::seconds(timeout_s)) + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + *elapsed_ms = (long)std::chrono::duration_cast( + clk::now() - t0) + .count(); + if (!done.load()) + return false; + t.join(); + return true; +} + +int main(int argc, char **argv) { + uint16_t vid = 0x0bda, pid = 0xb812; + int channel = 6, stop_at_ms = 500, rx_ms = 4000, timeout_s = 20; + std::string mode = "initstop"; + bool do_reset = true; + for (int i = 1; i < argc; i++) { + if (!std::strcmp(argv[i], "--vid") && i + 1 < argc) + vid = (uint16_t)std::strtoul(argv[++i], nullptr, 0); + else if (!std::strcmp(argv[i], "--pid") && i + 1 < argc) + pid = (uint16_t)std::strtoul(argv[++i], nullptr, 0); + else if (!std::strcmp(argv[i], "--channel") && i + 1 < argc) + channel = std::atoi(argv[++i]); + else if (!std::strcmp(argv[i], "--stop-at-ms") && i + 1 < argc) + stop_at_ms = std::atoi(argv[++i]); + else if (!std::strcmp(argv[i], "--rx-ms") && i + 1 < argc) + rx_ms = std::atoi(argv[++i]); + else if (!std::strcmp(argv[i], "--timeout-s") && i + 1 < argc) + timeout_s = std::atoi(argv[++i]); + else if (!std::strcmp(argv[i], "--mode") && i + 1 < argc) + mode = argv[++i]; + else if (!std::strcmp(argv[i], "--no-reset")) + do_reset = false; + else { + std::fprintf(stderr, "bad arg %s\n", argv[i]); + return 2; + } + } + + auto logger = std::make_shared(); + libusb_context *ctx = nullptr; + if (libusb_init(&ctx) < 0) { + std::fprintf(stderr, "libusb_init failed\n"); + return 3; + } + devourer::DeviceSession session(logger); + session.adopt_context(ctx); + libusb_device_handle *handle = libusb_open_device_with_vid_pid(ctx, vid, pid); + if (!handle) { + std::fprintf(stderr, "no adapter %04x:%04x\n", vid, pid); + return 3; + } + std::shared_ptr lock; + if (devourer::claim_interface_then_reset( + handle, devourer::find_wifi_interface(handle), logger, do_reset, lock) != + 0) { + session.adopt_handle(handle); + return 3; + } + session.adopt_handle(handle); + session.adopt_lock(lock); + devourer::DeviceConfig cfg; + WiFiDriver driver(logger); + std::unique_ptr owned = driver.CreateRadio(handle, ctx, lock, cfg); + if (!owned) { + std::fprintf(stderr, "CreateRadio failed\n"); + return 3; + } + session.adopt_device(std::move(owned)); + IRadio *const dev = session.device(); + const SelectedChannel chan{.Channel = (uint8_t)channel, + .ChannelOffset = 0, + .ChannelWidth = CHANNEL_WIDTH_20}; + + std::atomic frames{0}; + auto on_pkt = [&frames](const Packet &) { frames++; }; + + auto fail_hung = [&](const char *what, long waited_ms) { + std::printf("RESULT mode=%s outcome=DEADLOCK stage=%s waited_ms=%ld " + "frames=%ld\n", + mode.c_str(), what, waited_ms, frames.load()); + std::fflush(stdout); + /* The RX thread is stuck inside the library; destructors would run the + * teardown under it. Drop the USB lock by hand and leave. */ + lock.reset(); + session.release_lock(); + std::_Exit(1); + }; + + if (mode == "stale") + dev->StopRxLoop(); /* the pre-Init stale request */ + + std::atomic init_done{false}; + std::thread init_thread([&]() { + dev->Init(on_pkt, chan); + init_done = true; + }); + const int first_wait = (mode == "initstop" || mode == "restart") ? stop_at_ms + : rx_ms; + std::this_thread::sleep_for(std::chrono::milliseconds(first_wait)); + const long frames_at_stop = frames.load(); + if (mode == "initstop" && init_done.load()) { + /* Init came back on its own before the scheduled stop: a failed bring-up + * (Kestrel returns normally on one) or a stop time past the loop's life. + * Nothing about the contract was exercised. */ + init_thread.join(); + std::printf("RESULT mode=initstop outcome=INIT_RETURNED_BEFORE_STOP " + "stop_at_ms=%d frames=%ld\n", + stop_at_ms, frames_at_stop); + return 1; + } + dev->StopRxLoop(); + long waited = 0; + if (!join_within(init_thread, init_done, timeout_s, &waited)) + fail_hung("init", waited); + const long init_return_ms = waited; + + if (mode == "restart") { + frames = 0; + std::atomic rx_done{false}; + std::thread rx_thread([&]() { + dev->StartRxLoop(on_pkt); + rx_done = true; + }); + std::this_thread::sleep_for(std::chrono::milliseconds(rx_ms)); + const long f = frames.load(); + const bool loop_still_running = !rx_done.load(); + dev->StopRxLoop(); + if (!join_within(rx_thread, rx_done, timeout_s, &waited)) + fail_hung("restart", waited); + const bool ok = loop_still_running && f > 0; + std::printf("RESULT mode=restart outcome=%s init_return_ms=%ld " + "restart_loop_ran=%d restart_frames=%ld restart_return_ms=%ld\n", + ok ? "OK" : "RESTART_REFUSED", init_return_ms, + loop_still_running, f, waited); + return ok ? 0 : 1; + } + if (mode == "stale") { + const bool ok = frames_at_stop > 0; + std::printf("RESULT mode=stale outcome=%s frames_during_init=%ld " + "init_return_ms=%ld\n", + ok ? "OK" : "INIT_RX_REFUSED", frames_at_stop, init_return_ms); + return ok ? 0 : 1; + } + /* The stop was honoured; now show the chip it interrupted is up. */ + frames = 0; + std::atomic rx_done{false}; + std::thread rx_thread([&]() { + dev->StartRxLoop(on_pkt); + rx_done = true; + }); + std::this_thread::sleep_for(std::chrono::milliseconds(rx_ms)); + const long verify_frames = frames.load(); + const bool loop_still_running = !rx_done.load(); + dev->StopRxLoop(); + if (!join_within(rx_thread, rx_done, timeout_s, &waited)) + fail_hung("verify", waited); + const bool ok = loop_still_running && verify_frames > 0; + std::printf("RESULT mode=initstop outcome=%s stop_at_ms=%d init_return_ms=%ld " + "frames_at_stop=%ld verify_frames=%ld\n", + ok ? "OK" : "NO_RX_AFTER_STOP", stop_at_ms, init_return_ms, + frames_at_stop, verify_frames); + return ok ? 0 : 1; +}