sensing: a busy window must not outlive its hardware session

docs/rx-spectrum-sensing.md

@@ -415,7 +415,8 @@ quick-connect decision has.
 ### A window is only about the dwell if nothing else touched it
 
 Three things spoil one, all measured, and each makes the reading come back
-invalid with a reason (`ChannelBusy::spoil`) rather than plausible:
+invalid with a reason (`ChannelBusy::spoil`) rather than plausible. A fourth —
+the session ending under it — is not a spoiler but a lifetime rule, below.
 
 | spoiler | Jaguar1 (11AC) | Jaguar3 (JGR3) | RTL8733B (JGR3) |
 |---|---|---|---|
@@ -439,6 +440,35 @@ fails loudly: `GetRxQuality()` calls `GetRxEnergy(with_nhm=true)`, so a
 consumer polling link quality inside its own survey dwell spoils it without
 touching the busy API at all.
 
+### A window does not outlive its hardware session
+
+`Stop()` forgets any armed window. Without that, one armed before a teardown
+stays reachable afterwards and the next retune's note stamps it `Retuned` — a
+reason earned by a session that no longer exists. The mechanism differs by
+die: on the RTL8733B the retune re-runs bring-up, which restores the flag
+`with_ccx` gates on; on Jaguar1/2/3 the retune does no bring-up at all and the
+window simply stays reachable because nothing ever clears `_brought_up`. The
+reading is invalid either way, so what a missing reset costs is the REASON —
+session end never produces a spoil value of its own, it forges another's.
+
+Measured with `tests/busy_window_probe.sh --mode revive` (arm, `Stop()`,
+retune, read), each die with its own reset removed and then restored:
+
+| sensor | without the reset | with it | reading, with it |
+|---|---|---|---|
+| RTL8812AU (Jaguar1) | `spoil=retuned` | `spoil=none` | no reading — `Stop()` powers the card down |
+| RTL8822BU (Jaguar2) | `spoil=retuned` | `spoil=none` | **valid, 2 ms window** — this `Stop()` only joins its runtime threads |
+| RTL8812CU (Jaguar3) | `spoil=retuned` | `spoil=none` | no reading — `Stop()` runs `rtw_hal_deinit()` |
+| RTL8733BU | `spoil=retuned` | `spoil=none` | no reading — and not because of teardown depth: this die has no sampled path at all, since it does not override `GetRxEnergy` |
+
+The Jaguar2 column is why the arm asserts the spoil REASON and not the
+reading's validity: an "invalid" assertion encodes one family's teardown depth
+as a contract and fails a correct backend. What holds everywhere is that no
+reason survives the session that ended.
+
+The contract is at `IRadio::ArmChannelBusy`; how far each generation's
+teardown goes is in its own `src/<gen>/CLAUDE.md`.
+
 ### Own transmission is carried, not corrected
 
 CLM counts receive-side deferral only, and a radio is deaf to the channel while

src/IRadio.h

@@ -617,7 +617,26 @@ class IRadio {
    * of the dwell. A window is spoiled by an NHM read (IRtlRadio::GetRxEnergy
    * with with_nhm, which re-arms the shared CCX engine), by a retune, and by
    * reading before it has elapsed; the reading then comes back INVALID rather
-   * than plausible-but-wrong. A spoiled or completed window is consumed by
+   * than plausible-but-wrong.
+   *
+   * A window also does not outlive its hardware session: Stop() forgets it.
+   * Without that a window armed before a teardown stays reachable afterwards
+   * — by different mechanisms on different backends, so clearing whatever
+   * flag guards the engine is not a substitute — and the next retune's note
+   * stamps it Retuned, a reason earned by a session that no longer exists.
+   * Measured on all four Realtek backends. The reading is invalid either
+   * way, so what a missing reset costs is the REASON, which is the whole
+   * point of the spoil field. A backend implementing this owes the reset;
+   * the per-generation guides record how far each teardown goes.
+   *
+   * What the reset does NOT close: it forgets a window armed BEFORE the
+   * teardown. Where nothing clears the flag the engine's own accessor gates
+   * on, an arm issued AFTER Stop() still succeeds against a chip that has
+   * been torn down — true of Jaguar1/2/3, whose gate is never cleared, and
+   * not of the RTL8733B, whose Stop() clears it. Closing it everywhere is a
+   * behaviour change on paths that flag also guards.
+   *
+   * A spoiled or completed window is consumed by
    * the read; a not-yet-elapsed one stays armed, so the caller reads again
    * at the end of its dwell instead of re-arming. Single control thread, like
    * every other control-plane entry point. */

src/IRtlRadio.h

@@ -245,10 +245,14 @@ class IRtlRadio : public IRadio {
   void busy_window_note_nhm_read() { _busy_window.note_nhm_read(); }
   void busy_window_note_retune() { _busy_window.note_retune(); }
 
-  /* Forget any armed window. Bring-up paths call it: a window armed before a
-   * re-Init describes a chip state that no longer exists, and leaving it
-   * armed would make the next unrelated GetChannelBusy() take the armed
-   * branch and report a stale period as if it were its own dwell. */
+  /* Forget any armed window. Bring-up AND teardown paths call it: a window
+   * armed before a re-Init — or before a Stop() — describes a chip state that
+   * no longer exists, and leaving it armed would make the next unrelated
+   * GetChannelBusy() take the armed branch and report a stale period as if it
+   * were its own dwell. Stop() is the half that is easy to miss, because
+   * clearing the flag with_ccx gates on does not stand in for it: on a
+   * backend whose retune re-runs bring-up, that flag comes straight back.
+   * See IRadio::ArmChannelBusy for the contract. */
   void busy_window_reset() { _busy_window = devourer::ClmWindow{}; }
 
   /* Serialises the CCX engine: the armed window's state, the arm/read, and

src/jaguar1/CLAUDE.md

@@ -39,6 +39,18 @@ methods), `RadioManagementModule` (channel/BW/TX power, up to 4 RF paths),
 
 ## Teardown
 
+**`Stop()` forgets any armed busy window** — the rule, and the residual it
+does not close, are at `IRadio::ArmChannelBusy`, the one declaration site
+where they can be kept true. What is specific to this die:
+
+Measured on an RTL8812AU with the reset removed: arm, `Stop()`, retune, read
+reports `spoil=retuned` — a reason earned by a session that no longer exists;
+with it, `spoil=none` and no reading (this die's `Stop()` powers the card
+down). The reset sits ABOVE the `teardown_power_down=0` early return, so the
+"leave the chip powered" path forgets the window too. It takes the CCX lock
+alone: this generation has no family-wide register lock to order against
+(`_port0_mu` is narrower and never taken under the CCX lock).
+
 `RtlJaguarDevice::Stop()` and the destructor run `HalModule::rtw_hal_deinit()`:
 halt the MAC engines (`REG_CR`, `REG_RCR`), then the die's card-disable power
 sequence via the existing `HalPwrSeqCmdParsing` (`rtl8812_card_disable_flow` /

src/jaguar1/RtlJaguarDevice.cpp

@@ -2395,6 +2395,28 @@ bool RtlJaguarDevice::NetDevOpen(SelectedChannel selectedChannel) {
  * Best-effort: a chip that already dropped off the bus makes the writes fail,
  * which is fine on a teardown path. */
 void RtlJaguarDevice::Stop() {
+  /* The armed window dies with the session. Nothing else forgets it: this
+   * generation's with_ccx gates on _brought_up, which Stop() does not clear,
+   * so a window armed before a Stop stays visible afterwards and the next
+   * retune's note hands the caller a spoil reason earned by a session that no
+   * longer exists. Measured on an RTL8812AU with this reset removed: an
+   * arm/Stop/retune/read sequence reports spoil=retuned; with it, none.
+   * Scoped; nothing below takes the CCX lock. This generation has no
+   * FAMILY-WIDE register lock to order against (it has _port0_mu, a
+   * narrower one over the port0/TSF block, which is never taken under the
+   * CCX lock).
+   *
+   * What this does NOT close: no lock spans this Stop(), so a concurrent
+   * ArmChannelBusy can still land after the reset and during teardown, and
+   * with_ccx gates on _brought_up, which nothing here clears — so an arm
+   * issued AFTER a Stop still succeeds against a torn-down chip.
+   * ArmChannelBusy is single-control-thread by contract (IRadio.h); closing
+   * the rest means clearing _brought_up, which gates other paths. The
+   * contract and this residual are both at IRadio::ArmChannelBusy. */
+  {
+    std::lock_guard<std::mutex> ccx(busy_window_mutex());
+    busy_window_reset();
+  }
   _device.quiesce_tx();
   if (!_cfg.tuning.teardown_power_down) {
     _logger->info("Jaguar1: Stop() leaving the chip powered "

src/jaguar2/CLAUDE.md

@@ -72,6 +72,18 @@ The 8822B/8821C descriptor `TXPWR_OFSET` is a hardware LUT
 
 ## CCX energy sensing (`clm` / `nhm_env`)
 
+**`Stop()` forgets any armed busy window** — the rule, and the residual it
+does not close, are at `IRadio::ArmChannelBusy`, the one declaration site
+where they can be kept true. What is specific to this die:
+
+Measured on an RTL8822BU with the reset removed: arm, `Stop()`, retune, read
+reports `spoil=retuned`; with it, `spoil=none`. Note this `Stop()` does NOT
+tear the chip down — it only joins `stop_pwrtrack()`/`stop_dig()` — so after
+the reset the sampled path still answers, with a live 2 ms window. That is
+why the on-air `revive` arm asserts the spoil REASON rather than the reading's
+validity: asserting "invalid" would encode another family's teardown depth as
+a contract and fail this one. Neither joined thread takes the CCX lock.
+
 `GetRxEnergy(with_nhm=true)` runs the shared CCX window (`src/NhmReader.h`) on
 the 11AC register map; on-air validated on an RTL8822BU.
 

src/jaguar2/RtlJaguar2Device.cpp

@@ -2013,6 +2013,30 @@ bool RtlJaguar2Device::WriteTsf(uint64_t tsf) {
 }
 
 void RtlJaguar2Device::Stop() {
+  /* The armed window dies with the session. Nothing else forgets it:
+   * with_ccx gates on _brought_up, which Stop() does not clear, so a window
+   * armed before a Stop stays visible afterwards and the next retune's note
+   * hands the caller a spoil reason earned by a session that no longer
+   * exists. Measured on an RTL8822BU with this reset removed: an
+   * arm/Stop/retune/read sequence reports spoil=retuned; with it, none.
+   *
+   * Note this Stop does NOT tear the chip down — it only joins the runtime
+   * threads below — so after the reset the sampled path still answers, with
+   * a live 2 ms window. That is why the on-air `revive` arm asserts the spoil
+   * REASON rather than the reading's validity. Scoped; neither joined thread
+   * takes the CCX lock.
+   *
+   * What this does NOT close: no lock spans this Stop(), so a concurrent
+   * ArmChannelBusy can still land after the reset and during teardown, and
+   * with_ccx gates on _brought_up, which nothing here clears — so an arm
+   * issued AFTER a Stop still succeeds against a torn-down chip.
+   * ArmChannelBusy is single-control-thread by contract (IRadio.h); closing
+   * the rest means clearing _brought_up, which gates other paths. The
+   * contract and this residual are both at IRadio::ArmChannelBusy. */
+  {
+    std::lock_guard<std::mutex> ccx(busy_window_mutex());
+    busy_window_reset();
+  }
   stop_pwrtrack();
   stop_dig();
 }

src/jaguar3/CLAUDE.md

@@ -106,6 +106,17 @@ same TSSI reshape as its offset slope).
 
 ## CCX energy sensing (`clm` / `nhm_env`)
 
+**`Stop()` forgets any armed busy window** — the rule, and the residual it
+does not close, are at `IRadio::ArmChannelBusy`, the one declaration site
+where they can be kept true. What is specific to this die:
+
+Measured on an RTL8812CU with the reset removed: arm, `Stop()`, retune, read
+reports `spoil=retuned`; with it, `spoil=none` and no reading (`Stop()` runs
+`rtw_hal_deinit()`). The reset sits OUTSIDE `_reg_mu`, unlike the RTL8733B's,
+and deliberately: `Stop()` joins the coex thread, and that thread takes
+`_reg_mu`, so holding it across the join would deadlock. The coex loop never
+takes the CCX lock, which is what makes this ordering safe.
+
 **An armed busy window (`ArmChannelBusy`) is DESTROYED by an NHM read on this
 map.** Measured on an RTL8812CU: a clean 240 ms window read 60.4-61.6% under
 load, while the same window with one `GetRxEnergy(with_nhm=true)` mid-way came

src/jaguar3/RtlJaguar3Device.cpp

@@ -747,6 +747,28 @@ void RtlJaguar3Device::apply_replay_wseq() {
 /* Clean shutdown — see IRadio::Stop. Best-effort: a chip that already
  * dropped off the bus will make the de-init writes fail, which is fine. */
 void RtlJaguar3Device::Stop() {
+  /* The armed window dies with the session. Nothing else forgets it:
+   * with_ccx gates on _brought_up, which Stop() does not clear, so a window
+   * armed before a Stop stays visible afterwards and the next retune's note
+   * hands the caller a spoil reason earned by a session that no longer
+   * exists. Measured on an RTL8812CU with this reset removed: an
+   * arm/Stop/retune/read sequence reports spoil=retuned; with it, none.
+   * Scoped, and deliberately NOT under _reg_mu: Stop() joins the coex thread
+   * below, that thread takes _reg_mu, and holding it across the join would
+   * deadlock. Taking the CCX lock alone is safe here because the coex loop
+   * never takes it.
+   *
+   * What this does NOT close: no lock spans this Stop(), so a concurrent
+   * ArmChannelBusy can still land after the reset and during teardown, and
+   * with_ccx gates on _brought_up, which nothing here clears — so an arm
+   * issued AFTER a Stop still succeeds against a torn-down chip.
+   * ArmChannelBusy is single-control-thread by contract (IRadio.h); closing
+   * the rest means clearing _brought_up, which gates other paths. The
+   * contract and this residual are both at IRadio::ArmChannelBusy. */
+  {
+    std::lock_guard<std::mutex> ccx(busy_window_mutex());
+    busy_window_reset();
+  }
   _coex_stop = true;
   if (_coex_thread.joinable())
     _coex_thread.join();

src/rtl8733b/CLAUDE.md

@@ -480,12 +480,21 @@ and the retune note hands the caller a `Retuned` spoil earned by a hardware
 session that no longer exists: invalid either way, but the reason would be a
 lie. The reset is scoped under the `_reg_mu` `Stop()` already holds.
 
-Jaguar1/2/3 share this hole and have it worse — none of their `Stop()`
-implementations resets the window either, and unlike this backend none of them
-clears the flag their `with_ccx` gates on, so a `GetChannelBusy()` straight
-after `Stop()` reads CCX registers on a deinitialised chip with no retune
-needed. Not fixed here; noted so the asymmetry is not mistaken for an 8733B
-quirk.
+All of that is about a window armed BEFORE the Stop. For an arm issued
+*after* one, the flag does its job, and this backend closes both sides of the
+hazard: SEQUENTIALLY, `Stop()` clears `_phy_ready`, which is what `with_ccx`
+gates on, so a later `ArmChannelBusy()` is refused rather than arming a
+torn-down chip; CONCURRENTLY, `Stop()` holds the recursive `_reg_mu` across
+its whole body and `with_ccx` takes that lock first, so an arm racing the
+teardown blocks instead of slipping in after the reset.
+
+Jaguar1/2/3 reset the window in `Stop()` the same way but stop there — they
+have NEITHER half: no `Stop()` clears `_brought_up`, and none holds a
+register lock across its teardown (Jaguar1 has no family-wide one at all).
+So on those an arm issued after a Stop still succeeds against a torn-down
+chip, and one racing the teardown can still slip in. Each Jaguar guide
+records its own generation's teardown, the race is in each `Stop()`'s own
+comment, and `IRadio::ArmChannelBusy` carries the rule itself.
 
 Retune notes live in `SetMonitorChannel` and `FastRetune`, both **scoped**:
 `FastRetune` calls `SetMonitorChannel` on its declined path while already

tests/busy_window_probe.cpp

@@ -18,6 +18,15 @@
  *            the counter runs across the channel change.
  *   early    arm, read immediately. Must be INVALID (spoil=not-elapsed)
  *            rather than the previous window's latched value.
+ *   revive   arm, Stop(), retune, read. The window must not outlive its own
+ *            hardware session. A backend that does not forget it here hands
+ *            back a spoil reason earned by a session that no longer exists —
+ *            measured as spoil=retuned on all four Realtek backends with the
+ *            reset removed. The assertion is on the REASON only: whether the
+ *            read is valid depends on how deeply that backend's Stop tears
+ *            the chip down, and on a Jaguar2 (whose Stop only joins its
+ *            runtime threads) the sampled path answers with a live 2 ms
+ *            window. Not runnable on MediaTek: Stop() closes the device.
  *   txsess   arm, TRANSMIT inside the window, read. Must stay valid and be
  *            flagged own_tx: Realtek reads low (the receiver is deaf while
  *            the PA is up), MediaTek reads high (it counts own airtime).
@@ -177,7 +186,8 @@ int main(int argc, char **argv) {
       std::fprintf(stderr,
                    "usage: %s [--vid N --pid N] [--channel N] [--other N] "
                    "[--reps N] [--window-ms N] [--rx] "
-                   "[--mode sampled|window|interrupt|retune|early|txsess]\n",
+                   "[--mode sampled|window|interrupt|quality|race|retune|early|"
+                   "revive|stale|txsess]\n",
                    argv[0]);
       return 2;
     }
@@ -271,6 +281,18 @@ int main(int argc, char **argv) {
     nap(200);
   }
 
+  /* `revive` tears the session down and brings it back. With --rx the Realtek
+   * Init is running on a DETACHED thread (it ends in a blocking StartRxLoop),
+   * so Stop() and SetMonitorChannel would run underneath it — the same
+   * use-after-free the cleanup path above exists to avoid, and the sequence
+   * RtlJaguar3Device::InitWrite refuses outright. Refuse rather than wedge. */
+  if (mode == "revive" && rx_on) {
+    devourer::Ev(g_ev, "busy.skip")
+        .t()
+        .f("why", "revive cannot run with the RX loop live");
+    return finish(5);
+  }
+
   const uint32_t window_us = static_cast<uint32_t>(window_ms) * 1000u;
 
   for (int i = 1; i <= reps; i++) {
@@ -326,6 +348,40 @@ int main(int argc, char **argv) {
       nap(wait_ms / 2);
     } else if (mode == "early") {
       nap(5);
+    } else if (mode == "revive") {
+      /* The window must not survive its own hardware session.
+       *
+       * Stop() ends the session, but that alone does not protect an armed
+       * window, and the mechanism differs by backend. On the RTL8733B,
+       * SetMonitorChannel re-runs bring_up_to_phy(), which sets the flag
+       * with_ccx gates on back to true. On Jaguar1/2/3 the retune does no
+       * bring-up at all — there the window stays reachable simply because
+       * nothing ever clears `_brought_up`. Either way a backend that does
+       * not reset the window in Stop() hands the next read a spoil reason
+       * earned by a session that no longer exists, and the retune note below
+       * is what stamps it.
+       *
+       * The harness asserts the REASON only. Whether the read is valid
+       * depends on how deeply that backend's Stop tears the chip down:
+       * measured false on the RTL8733B, Jaguar1 and Jaguar3, and TRUE with a
+       * 2 ms window on a Jaguar2, whose Stop only joins its runtime
+       * threads.
+       *
+       * Read `5/5 spoil=none` as ONE confirmation, not five. This arm sits
+       * inside the rep loop, so reps 2..N arm a chip the previous rep's
+       * Stop() already tore down — which succeeds only because of the
+       * residual this very comment is about (nothing clears the gate
+       * with_ccx reads). Only rep 1 exercises the intended live-session
+       * sequence. The negative control is what discriminates: with the
+       * reset removed every rep reports spoil=retuned. */
+      nap(wait_ms / 2);
+      dev->Stop();
+      nap(50);
+      dev->SetMonitorChannel(
+          SelectedChannel{.Channel = static_cast<uint8_t>(channel),
+                          .ChannelOffset = 0,
+                          .ChannelWidth = CHANNEL_WIDTH_20});
+      nap(wait_ms);
     } else if (mode == "stale") {
       /* Let this window finish and consume it, so the result register holds a
        * completed measurement; then arm again and read at once. */