Quiesce TX before libusb teardown (#281); fix Kestrel CCK + wide-bandwidth RX (#343)

.github/workflows/cmake-multi-platform.yml

@@ -200,6 +200,34 @@ jobs:
         working-directory: build
         run: ctest --output-on-failure
 
+  # Sanitized build of the whole tree (library + demos + selftests) with the
+  # headless suite run under ASan/UBSan. Leak detection is off here: the
+  # selftests are pure computation and never open a device, so the only
+  # allocations still live at exit would come from libusb in demo code this
+  # job never runs — leak triage belongs to the on-hardware runs.
+  build-sanitizers:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install dependency libraries
+        run: sudo apt install libusb-1.0-0-dev
+
+      - name: Configure (ASan + UBSan)
+        run: >
+          cmake -B build-asan -DCMAKE_BUILD_TYPE=RelWithDebInfo
+          -DDEVOURER_SANITIZE=address+undefined -S ${{ github.workspace }}
+
+      - name: Build (ASan + UBSan)
+        run: cmake --build build-asan -j
+
+      - name: Test (ASan + UBSan)
+        working-directory: build-asan
+        env:
+          ASAN_OPTIONS: detect_leaks=0
+          UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1
+        run: ctest --output-on-failure
+
   # Invalid option combinations must fail at configure time, not silently
   # produce a broken binary.
   reject-bad-configs:

.gitignore

@@ -1,5 +1,7 @@
 compile_commands.json
 build
+# Out-of-tree build dirs: build-asan (DEVOURER_SANITIZE), per-config trees.
+build-*/
 .cache
 __pycache__/
 *.pyc

CLAUDE.md

@@ -102,9 +102,21 @@ group exports a PUBLIC `DEVOURER_HAVE_*` define; sites referencing a dropped
 group sit behind `#if defined(DEVOURER_HAVE_*)`, and the factory returns
 `nullptr` (logs) for a chip whose support isn't built.
 
+`DEVOURER_SANITIZE=address|address+undefined|thread` (default off) builds the
+library, demos and selftests instrumented; the vendored halbb/halrf C is
+exempted from UBSan (Realtek's sources are full of unaligned loads and signed
+shifts — noise we would never act on). MSVC has AddressSanitizer only and
+rejects the other values rather than silently degrading. The demos are worth
+running under it on hardware, not just `ctest`: the lifetime bugs it finds
+(teardown, in-flight URBs) need a real device —
+`tests/tx_teardown_asan.sh` (max-duty TX killed mid-flight, incl. a real
+VBUS-cut wedge) and `tests/teardown_gen_sanity.sh` (every plugged generation
+init + teardown).
+
 CI (`.github/workflows/cmake-multi-platform.yml`): GCC/Clang/MSVC ×
 Ubuntu/macOS/Windows matrix, a `build-mingw` job, a `build-configs` matrix over
-each per-chip subset, and `reject-bad-configs` for the invalid option combos.
+each per-chip subset, `build-sanitizers` (ASan+UBSan `ctest`), and
+`reject-bad-configs` for the invalid option combos.
 `ctest` runs in every job (headless selftests — math guards + the
 `stream_stdin_binary` framing round-trip). Hardware testing is out-of-band.
 
@@ -509,6 +521,16 @@ to the factory. `examples/rx/main.cpp` is the canonical boilerplate;
 `devourer::claim_interface_then_reset` (src/UsbOpen.h) is the recommended
 open path (advisory per-adapter lock before reset).
 
+Owning libusb means owning the **teardown order**: destroy the `IRtlDevice`
+first, then release the interface, close the handle, and only then
+`libusb_exit`. The device is what quiesces TX (`IRtlDevice::Stop`, and the
+destructor as a backstop: Jaguar1's async bulk-OUT URBs must be cancelled and
+reaped while the context still exists), so tearing libusb down first is a
+crash, not a leak — and only under enough TX load to keep URBs outstanding at
+exit. `examples/common/DeviceSession.h` is the demos' RAII holder for exactly
+that order; the transport logs a diagnostic naming this if it is destroyed
+with TX still in flight.
+
 **Chip identity is resolved at construction** from the `SYS_CFG2` chip-id +
 USB PID. `CreateRtlDevice` returns an `IRtlDevice` (`Init` = bring-up + RX
 loop; `InitWrite` = TX bring-up; `StartRxLoop` = blocking RX worker on an

CMakeLists.txt

@@ -106,6 +106,53 @@ if(DEVOURER_PCIE)
     endif()
 endif()
 
+# ---------------------------------------------------------------------------
+# Sanitizers. Off by default; the value applies to the library, the examples
+# and the selftests alike, so `ctest` and an on-hardware demo run share one
+# instrumented build. The vendored halbb/halrf C is exempted from UBSan below
+# (unaligned loads / signed shifts are pervasive in Realtek's sources and are
+# not ours to fix).
+#   cmake -S . -B build-asan -DDEVOURER_SANITIZE=address+undefined
+# ---------------------------------------------------------------------------
+set(DEVOURER_SANITIZE "" CACHE STRING
+    "Sanitizer build: address, address+undefined, thread (empty = off)")
+set_property(CACHE DEVOURER_SANITIZE PROPERTY STRINGS
+    "" address address+undefined thread)
+if(NOT DEVOURER_SANITIZE STREQUAL "")
+    if(MSVC)
+        # MSVC ships AddressSanitizer only — no UBSan, no TSan, no LSan.
+        # Degrade loudly rather than silently building something weaker than
+        # the caller asked for.
+        if(NOT DEVOURER_SANITIZE STREQUAL "address")
+            message(FATAL_ERROR
+                "DEVOURER_SANITIZE=${DEVOURER_SANITIZE} is unsupported on MSVC — "
+                "it implements /fsanitize=address only. Use "
+                "-DDEVOURER_SANITIZE=address, or build with GCC/Clang for "
+                "undefined/thread.")
+        endif()
+        add_compile_options(/fsanitize=address)
+        # ASan is incompatible with the runtime checks and with incremental
+        # linking; both are on by default in Debug.
+        add_compile_options($<$<CONFIG:Debug>:/Oy->)
+        add_link_options(/INCREMENTAL:NO)
+    else()
+        if(DEVOURER_SANITIZE STREQUAL "address")
+            set(_devourer_san "address")
+        elseif(DEVOURER_SANITIZE STREQUAL "address+undefined")
+            set(_devourer_san "address,undefined")
+        elseif(DEVOURER_SANITIZE STREQUAL "thread")
+            set(_devourer_san "thread")
+        else()
+            message(FATAL_ERROR
+                "DEVOURER_SANITIZE=${DEVOURER_SANITIZE} — must be one of "
+                "address, address+undefined, thread (or empty).")
+        endif()
+        add_compile_options(-fsanitize=${_devourer_san} -fno-omit-frame-pointer -g)
+        add_link_options(-fsanitize=${_devourer_san})
+    endif()
+    message(STATUS "devourer: sanitizer build (${DEVOURER_SANITIZE})")
+endif()
+
 # Find pkg-config and then use it to locate libusb.
 find_package(PkgConfig REQUIRED)
 pkg_check_modules(libusb REQUIRED IMPORTED_TARGET libusb-1.0)
@@ -393,6 +440,13 @@ if(DEVOURER_KESTREL)
         set(_kestrel_vendor_cflags /W0 /Gy)
     else()
         set(_kestrel_vendor_cflags -w -ffunction-sections -fdata-sections)
+        # Realtek's sources are full of unaligned loads, signed shifts and
+        # out-of-range enum stores. They are vendored verbatim (edit the
+        # extractors, never these files), so UBSan on them reports noise we
+        # would never act on and drowns the findings from our own code.
+        if(DEVOURER_SANITIZE MATCHES "undefined")
+            list(APPEND _kestrel_vendor_cflags -fno-sanitize=undefined)
+        endif()
     endif()
 
     file(GLOB KESTREL_HALBB_C "${CMAKE_CURRENT_SOURCE_DIR}/hal/halbb/g6/vendor/*.c")
@@ -488,6 +542,8 @@ if(DEVOURER_KESTREL)
         examples/kestrelprobe/main.cpp
     )
     target_link_libraries(kestrelprobe PUBLIC devourer PRIVATE PkgConfig::libusb)
+    # DeviceSession.h (shared teardown-order holder) lives in examples/common/.
+    target_include_directories(kestrelprobe PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/examples/common)
 endif()
 
 # reglat — register round-trip latency microbench (USB ctrl-xfer vs PCIe MMIO).
@@ -678,6 +734,17 @@ target_link_libraries(ToneMaskSelftest PRIVATE devourer)
 
 add_test(NAME tone_mask_math COMMAND ToneMaskSelftest)
 
+# Headless guard for the TX quiesce seam (IRtlTransport::quiesce_tx via
+# RtlAdapter): the explicit "stop TX and wait it out" call every device makes
+# before anything is released. UsbTransport's cancel/drain is validated on
+# hardware under ASan — the test header says what it does and does not cover.
+add_executable(TxQuiesceSelftest
+    tests/tx_quiesce_selftest.cpp
+)
+target_link_libraries(TxQuiesceSelftest PRIVATE devourer PkgConfig::libusb)
+
+add_test(NAME tx_quiesce_seam COMMAND TxQuiesceSelftest)
+
 # Headless guard for the active idle-noise-floor math (src/NoiseFloorMath.h): the
 # phydm sign/pwdb helpers the Jaguar1 0x0FA0 debug-port measurement uses. Pure
 # math, header-only.

examples/chanmig/main.cpp

@@ -34,6 +34,7 @@
 #include <thread>
 #include <vector>
 
+#include "DeviceSession.h"
 #include "RadiotapBuilder.h"
 #include "RxPacket.h"
 #include "SignalStop.h"
@@ -455,6 +456,11 @@ int main(int argc, char **argv) {
   g_ev = &logger->events();
   install_devourer_signal_handlers();
 
+  /* Owns the teardown order (device -> interface -> handle -> context; see
+   * DeviceSession.h). Declared before every thread below, so the threads are
+   * joined before the adapter is released. */
+  devourer::DeviceSession session{logger};
+
   std::string role;
   for (int i = 1; i + 1 < argc; i++)
     if (std::strcmp(argv[i], "--role") == 0)
@@ -508,36 +514,43 @@ int main(int argc, char **argv) {
   libusb_context *ctx = nullptr;
   if (libusb_init(&ctx) < 0)
     return 1;
+  session.adopt_context(ctx);
   static constexpr uint16_t kPids[] = {0x8812, 0xc812, 0xa81a, 0x881a,
                                        0xc82c, 0xe822, 0x8813};
   UsbPick pick;
   libusb_device_handle *handle = open_selected_usb(
       ctx, logger, kPids, sizeof(kPids) / sizeof(kPids[0]), &pick);
-  if (!handle) {
-    libusb_exit(ctx);
+  if (!handle)
     return 1;
-  }
   std::shared_ptr<devourer::UsbDeviceLock> lock;
   if (devourer::claim_interface_reset_reopen(
           ctx, handle, logger, std::getenv("DEVOURER_SKIP_RESET") == nullptr,
           lock) != 0) {
-    libusb_close(handle);
-    libusb_exit(ctx);
+    /* The claim failed, so nothing owns the handle yet — hand it to the
+     * session purely so the unwind closes it. */
+    session.adopt_handle(handle);
     return 1;
   }
+  session.adopt_handle(handle);
+  session.adopt_lock(lock);
   /* Jaguar3 needs the RX filters opened at bring-up for reliable duplex. */
 #ifdef _WIN32
   _putenv_s("DEVOURER_TX_WITH_RX", "thread");
 #else
   ::setenv("DEVOURER_TX_WITH_RX", "thread", 1);
 #endif
   WiFiDriver driver(logger);
-  auto dev = driver.CreateRtlDevice(handle, ctx, lock, devourer_config_from_env());
-  if (!dev) {
+  auto owned_device =
+      driver.CreateRtlDevice(handle, ctx, lock, devourer_config_from_env());
+  if (!owned_device) {
     logger->error("no driver for this chip");
     return 1;
   }
-  g_dev = dev.get();
+  /* The session owns the device from here: it is what guarantees the device
+   * (and its in-flight TX) dies before libusb does. */
+  session.adopt_device(std::move(owned_device));
+  IRtlDevice *const dev = session.device();
+  g_dev = dev;
 
   Ev(*g_ev, "migrate.id").t().f("role", role.c_str())
       .f("chip", pick.pid).f("source", source.str().c_str())
@@ -726,8 +739,6 @@ int main(int argc, char **argv) {
     g_dev = nullptr;
   }
   dev->Stop();
-  libusb_release_interface(handle, 0);
-  libusb_close(handle);
-  libusb_exit(ctx);
+  session.close();
   return 0;
 }

examples/chanscout/main.cpp

@@ -43,6 +43,7 @@
 #include <thread>
 #include <vector>
 
+#include "DeviceSession.h"
 #include "RxPacket.h"
 #include "SignalStop.h"
 #include "UsbOpen.h"
@@ -172,6 +173,11 @@ int main() {
   g_ev = &logger->events();
   install_devourer_signal_handlers();
 
+  /* Owns the teardown order (device -> interface -> handle -> context; see
+   * DeviceSession.h). Declared before the RX thread below, so it is joined
+   * before the adapter is released. */
+  devourer::DeviceSession session{logger};
+
   /* --- plan --- */
   const char *plan_env = std::getenv("DEVOURER_SCOUT_PLAN");
   cm::ScanPlanConfig cfg;
@@ -236,6 +242,7 @@ int main() {
   int rc = libusb_init(&ctx);
   if (rc < 0)
     return rc;
+  session.adopt_context(ctx);
   libusb_set_option(ctx, LIBUSB_OPTION_LOG_LEVEL,
                     std::getenv("DEVOURER_USB_DEBUG") ? LIBUSB_LOG_LEVEL_DEBUG
                                                       : LIBUSB_LOG_LEVEL_WARNING);
@@ -250,29 +257,33 @@ int main() {
   UsbPick pick;
   libusb_device_handle *handle = open_selected_usb(
       ctx, logger, kPids, sizeof(kPids) / sizeof(kPids[0]), &pick);
-  if (handle == nullptr) {
-    libusb_exit(ctx);
+  if (handle == nullptr)
     return 1;
-  }
   std::shared_ptr<devourer::UsbDeviceLock> usb_lock;
   rc = devourer::claim_interface_reset_reopen(
       ctx, handle, logger, std::getenv("DEVOURER_SKIP_RESET") == nullptr,
       usb_lock);
   if (rc != 0) {
-    if (handle != nullptr)
-      libusb_close(handle);
-    libusb_exit(ctx);
+    /* The claim failed, so nothing owns the handle yet — hand it to the
+     * session purely so the unwind closes it. */
+    session.adopt_handle(handle);
     return 1;
   }
+  session.adopt_handle(handle);
+  session.adopt_lock(usb_lock);
 
   WiFiDriver driver(logger);
-  auto dev = driver.CreateRtlDevice(handle, ctx, usb_lock,
-                                    devourer_config_from_env());
-  if (!dev) {
+  auto owned_device = driver.CreateRtlDevice(handle, ctx, usb_lock,
+                                             devourer_config_from_env());
+  if (!owned_device) {
     logger->error("No driver for this chip in this build — exiting");
     return 1;
   }
-  devourer::emit_adapter_caps(*g_ev, dev.get());
+  /* The session owns the device from here: it is what guarantees the device
+   * (and its in-flight TX) dies before libusb does. */
+  session.adopt_device(std::move(owned_device));
+  IRtlDevice *const dev = session.device();
+  devourer::emit_adapter_caps(*g_ev, dev);
   const devourer::AdapterCaps caps = dev->GetAdapterCaps();
 
   /* Stable scout identity = the physical binding + silicon (FNV-1a). This is
@@ -349,7 +360,7 @@ int main() {
   cm::ScanScheduler sched(cfg);
 
   /* --- RX loop on a worker thread (rxdemo sweep pattern) --- */
-  IRtlDevice *devp = dev.get();
+  IRtlDevice *devp = dev;
   const cm::ScanScheduler::DwellPlan first = sched.next(steady_ms());
   std::thread rx([devp, first, &logger]() {
     try {
@@ -671,8 +682,6 @@ int main() {
   if (rx.joinable())
     rx.join();
   devp->Stop();
-  libusb_release_interface(handle, 0);
-  libusb_close(handle);
-  libusb_exit(ctx);
+  session.close();
   return sched.consecutive_failures() >= 15 ? 3 : 0;
 }