fix(session): enforce the stall-recovery ceiling where the wait is used

The Windows release build failed testStallWatchdogBacksOffRatherThanLooping
(11/124 checks) while Linux and macOS passed, so v0.1.1 never published.

Reconstructing the failure from the log rather than guessing: the reported
FAIL lines (line 329 first, then 327/329 alternating for the rest of the
capped-backoff loop, 11 of the loop's 12 checks) are produced by exactly one
behaviour, and the deliberately-broken build in this commit's verification
reproduced that log byte-for-byte on Linux -- the backoff ceiling engaged one
attempt LATE. Windows waited 32000ms once (the uncapped doubling of 16000ms)
before settling at the 30000ms ceiling. Every other candidate produces a
different count and a different order: an exact-equality boundary bug gives 6
failures, and a ceiling that never engages at all gives 8, neither matching.

That rules out the obvious suspect, a lossy duration conversion. There isn't
one, and there cannot be: `time_point<Clock, D1> + duration<D2>` yields
`time_point<Clock, common_type_t<D1, D2>>`, and converting that back to
`steady_clock::time_point` to store it in next_attempt_allowed_ only compiles
when the conversion is exact. If MSVC's steady_clock could not represent a
whole millisecond exactly, this file would not build there. All of the
watchdog's time arithmetic is exact integer arithmetic on every platform, and
the exact-equality comparison at the deadline is sound -- the Windows log
itself shows later polls firing at exactly their deadline.

What is left is `std::min(backoff_ * 2, max_backoff_)`: the one expression in
poll() that was not plain value arithmetic on a single type, returning a
*reference* bound, in the growing case, to a materialized temporary. So:

- The ceiling is now clamped where the wait is USED, not only where the
  backoff is grown. max_backoff_ is a promise about the longest gap between
  two recovery attempts, so it is enforced on the gap itself and holds for
  whatever backoff_ contains. Verified: with the growth step deliberately
  mis-capping exactly the way Windows did, the whole suite still passes --
  the fix does not depend on having correctly identified MSVC's mechanism.
- The doubling is an explicit compare-and-clamp instead of std::min, so no
  reference to a temporary is involved and the product is only computed when
  it cannot exceed the ceiling.

Both changes are provably no-ops on Linux and macOS, where backoff_ never
exceeded the ceiling in the first place.

Also pins the behaviour with a new regression test using a ceiling that is
NOT a power-of-two multiple of the timeout (1000 -> 2000 -> 4000 -> 5000),
which fails on the step the ceiling first binds rather than six 30-second
iterations later. 146 checks in test_session now, was 124; all 6 CTest suites
pass locally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

core/src/session_types.cpp

@@ -11,8 +11,6 @@ You may obtain a copy of the License at
 
 #include "stplugin/session_types.h"
 
-#include <algorithm>
-
 namespace stplugin {
 
 const char *describePixelFormat(PixelFormat format)
@@ -230,8 +228,27 @@ bool StallWatchdog::poll(std::chrono::steady_clock::time_point now)
     // 8s, ... up to max_backoff_ -- so a publisher that is genuinely gone
     // gets progressively less frequent toggles instead of one every 2
     // seconds for the rest of the show.
-    next_attempt_allowed_ = now + backoff_;
+    //
-    backoff_ = std::min(backoff_ * 2, max_backoff_);
+    // max_backoff_ is clamped HERE, where the wait is used, and not only
+    // where the backoff is grown. It is a promise about the longest gap
+    // between two recovery attempts, so it is enforced on the gap itself;
+    // that way the promise holds for whatever backoff_ happens to contain,
+    // rather than depending on every earlier growth step having clamped
+    // correctly. A capped release build on Windows got that one step wrong
+    // (the ceiling engaged one attempt late, so a single 32s gap slipped
+    // past the 30s ceiling), which is exactly the kind of drift this
+    // clamp-at-use makes unrepresentable.
+    const std::chrono::milliseconds wait = backoff_ < max_backoff_ ? backoff_ : max_backoff_;
+    next_attempt_allowed_ = now + wait;
+    // Double-and-clamp as plain value arithmetic on a single type. This was
+    // std::min(backoff_ * 2, max_backoff_), which returns a *reference* --
+    // bound, in the growing case, to the materialized `backoff_ * 2`
+    // temporary. That was the only expression in this function that was not
+    // a plain integer computation, and it is the one the Windows release
+    // build disagreed with the other two platforms about. Comparing before
+    // doubling also means the product is computed only when it cannot
+    // exceed max_backoff_, so no intermediate can overflow.
+    backoff_ = (wait > max_backoff_ / 2) ? max_backoff_ : wait * 2;
     return true;
 }
 

core/tests/test_session.cpp

@@ -339,6 +339,35 @@ void testStallWatchdogBacksOffRatherThanLooping()
     ST_ASSERT_EQ(w.attemptsThisStall(), 1);
 }
 
+// The ceiling has to engage on the FIRST attempt whose doubled backoff would
+// exceed it, not one attempt later -- a Windows release build got exactly
+// that step wrong (it waited 32s once before settling at the 30s ceiling),
+// which is why StallWatchdog::poll() clamps the wait where it is used rather
+// than trusting every growth step. A cap that is NOT a power-of-two multiple
+// of the timeout pins the clamp itself: 1000 -> 2000 -> 4000 -> 5000 (not
+// 8000, and not 4000 again), and 5000 forever after.
+void testStallWatchdogNeverWaitsLongerThanTheCeiling()
+{
+    const auto t0 = std::chrono::steady_clock::now();
+    StallWatchdog w(std::chrono::milliseconds(1000), std::chrono::milliseconds(5000));
+    w.setExpectingFrames(true, t0);
+
+    auto now = t0 + std::chrono::milliseconds(1000);
+    ST_ASSERT(w.poll(now));
+
+    // Expected gaps between consecutive attempts: 1000, 2000, 4000, then the
+    // ceiling for good. Each gap is checked on both sides of its boundary, so
+    // a gap that is even one millisecond too long or too short fails here.
+    const int expected_gaps[] = {1000, 2000, 4000, 5000, 5000, 5000, 5000};
+    int attempt = 1;
+    for (int gap : expected_gaps) {
+        ST_ASSERT(!w.poll(now + std::chrono::milliseconds(gap - 1)));
+        now += std::chrono::milliseconds(gap);
+        ST_ASSERT(w.poll(now));
+        ST_ASSERT_EQ(w.attemptsThisStall(), ++attempt);
+    }
+}
+
 // ---------------------------------------------------------------------------
 // Real SDK, failure paths only (no LiveKit server available headlessly)
 // ---------------------------------------------------------------------------
@@ -468,6 +497,7 @@ int main()
     testStallWatchdogFiresAfterThreshold();
     testStallWatchdogDoesNotFireWhenNotExpectingFrames();
     testStallWatchdogBacksOffRatherThanLooping();
+    testStallWatchdogNeverWaitsLongerThanTheCeiling();
 
     LiveKitSession::globalInitialize();
     testConnectRejectsIncompleteConfig();