fix(http): bound the WinHTTP response-header wait, and stop docs triggering builds #5

Merged
jknapp merged 3 commits from fix/winhttp-response-header-timeout into main 2026-09-10 01:54:16 +00:00
Owner

The failure

test_api_client's testPlatformBackendTimeout fails intermittently on Windows — roughly 2 runs in 6 — with both assertions failing together:

FAIL test_api_client.cpp:439: !response.ok()
FAIL test_api_client.cpp:440: elapsed.count() < 4000

A request with timeout_ms = 700 waited out a 5s server stall and returned 200 OK. Seen in job 5834 (2026-09-07) and job 5911 (2026-09-09), on code that passed on other runs — pre-existing and intermittent, not caused by a change.

1. The real bug

WinHttpSetTimeouts' receive parameter maps to WINHTTP_OPTION_RECEIVE_TIMEOUT, which Microsoft documents as a per-packet Winsock-layer read timeout — "applies to fetching each packet of data off the socket" (IWinHttpRequest::SetTimeouts).

The wait for the response headers is a different option — WINHTTP_OPTION_RECEIVE_RESPONSE_TIMEOUT, "the timeout value... to wait to receive all response headers to a request", default 90 seconds (Option flags) — which WinHttpSetTimeouts does not set.

So a server that accepts, reads the request, then stalls could block the calling thread for 90 seconds regardless of timeout_ms. That is exactly the "blocking an OBS thread indefinitely" failure this test exists to prevent. Now set explicitly, #ifdef-guarded so an older Windows SDK still builds.

What is not established: whether this fully explains the intermittency. The same documentation notes this timeout "is checked only when data is received from the socket", so neither option is a hard deadline — that needs a watchdog thread calling WinHttpCloseHandle, deliberately not built here. The fix stands on its own merits either way.

2. Evidence for the next run

The probe now runs 5 times and prints per attempt: elapsed ms, ok, status, requests_seen, and the backend's error string (which carries GetLastError). One CI run now yields a failure rate and an error code instead of one bit.

Each attempt gets a fresh loopback server — LoopbackServer handles one connection at a time on a single thread, so reusing it would leave attempts 2..n in the accept backlog, never accepted, which is a different scenario from the one that fails. (Caught by running it locally first: requests_seen stayed at 1 across all five.)

Verified on Linux: 5/5 attempts give up at ~701 ms.

3. Docs no longer trigger three-platform builds

build.yml gains paths-ignore for **.md, LICENSE, NOTICE, and the two release-only files. This is a full three-platform build behind a runner with capacity:1, and six fired for one afternoon of documentation edits. Nothing that feeds a build or a test is on the list.

Tradeoff, noted in the file: a docs-only push now shows no status rather than a green one. If a required-status check is ever added, these paths need reconsidering.

Verification

All 6 suites pass locally on Linux (test_api_client 25.4s with the 5-probe loop, within its 120s CTest timeout). The WinHTTP change cannot be compiled on this Linux host — CI is its first build, and the Windows job in this PR is the real check.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AzGnvQ6wfD7bw7PZN35ft9

## The failure `test_api_client`'s `testPlatformBackendTimeout` fails intermittently on Windows — roughly 2 runs in 6 — with both assertions failing together: ``` FAIL test_api_client.cpp:439: !response.ok() FAIL test_api_client.cpp:440: elapsed.count() < 4000 ``` A request with `timeout_ms = 700` waited out a 5s server stall and returned **200 OK**. Seen in job 5834 (2026-09-07) and job 5911 (2026-09-09), on code that passed on other runs — pre-existing and intermittent, not caused by a change. ## 1. The real bug `WinHttpSetTimeouts`' receive parameter maps to `WINHTTP_OPTION_RECEIVE_TIMEOUT`, which Microsoft documents as a **per-packet** Winsock-layer read timeout — *"applies to fetching each packet of data off the socket"* ([IWinHttpRequest::SetTimeouts](https://learn.microsoft.com/en-us/windows/win32/winhttp/iwinhttprequest-settimeouts)). The wait for the response **headers** is a different option — `WINHTTP_OPTION_RECEIVE_RESPONSE_TIMEOUT`, *"the timeout value... to wait to receive all response headers to a request"*, **default 90 seconds** ([Option flags](https://learn.microsoft.com/en-us/windows/win32/winhttp/option-flags)) — which `WinHttpSetTimeouts` does not set. So a server that accepts, reads the request, then stalls could block the calling thread for **90 seconds** regardless of `timeout_ms`. That is exactly the *"blocking an OBS thread indefinitely"* failure this test exists to prevent. Now set explicitly, `#ifdef`-guarded so an older Windows SDK still builds. **What is not established:** whether this fully explains the intermittency. The same documentation notes this timeout *"is checked only when data is received from the socket"*, so neither option is a hard deadline — that needs a watchdog thread calling `WinHttpCloseHandle`, deliberately not built here. The fix stands on its own merits either way. ## 2. Evidence for the next run The probe now runs 5 times and prints per attempt: elapsed ms, `ok`, status, `requests_seen`, and the backend's error string (which carries `GetLastError`). One CI run now yields a failure **rate** and an error code instead of one bit. Each attempt gets a **fresh** loopback server — `LoopbackServer` handles one connection at a time on a single thread, so reusing it would leave attempts 2..n in the accept backlog, never accepted, which is a different scenario from the one that fails. (Caught by running it locally first: `requests_seen` stayed at 1 across all five.) Verified on Linux: 5/5 attempts give up at ~701 ms. ## 3. Docs no longer trigger three-platform builds `build.yml` gains `paths-ignore` for `**.md`, `LICENSE`, `NOTICE`, and the two release-only files. This is a full three-platform build behind a runner with `capacity:1`, and **six** fired for one afternoon of documentation edits. Nothing that feeds a build or a test is on the list. Tradeoff, noted in the file: a docs-only push now shows *no* status rather than a green one. If a required-status check is ever added, these paths need reconsidering. ## Verification All 6 suites pass locally on Linux (`test_api_client` 25.4s with the 5-probe loop, within its 120s CTest timeout). **The WinHTTP change cannot be compiled on this Linux host — CI is its first build**, and the Windows job in this PR is the real check. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01AzGnvQ6wfD7bw7PZN35ft9
jknapp added 1 commit 2026-09-10 01:30:22 +00:00
fix(http): bound the WinHTTP response-header wait, and stop docs triggering builds
Build / macOS (macos-latest) (push) Successful in 50s
Build / Linux (ubuntu-24.04) (push) Successful in 1m14s
Build / macOS (macos-latest) (pull_request) Successful in 49s
Build / Linux (ubuntu-24.04) (pull_request) Successful in 1m16s
Build / Windows (windows-latest) (push) Failing after 3m55s
Build / Windows (windows-latest) (pull_request) Failing after 3m51s
8888e57d08
Two things, both prompted by an intermittent Windows CI failure in
test_api_client's testPlatformBackendTimeout: roughly 2 runs in 6, both its
assertions failed together, meaning a request with timeout_ms=700 waited out a
5s server stall and returned 200. Same failure on 2026-09-07 (job 5834) and
2026-09-09 (job 5911), on code that passed on other runs -- pre-existing and
intermittent, not caused by a change.

1. The real bug. `WinHttpSetTimeouts`' receive parameter maps to
   WINHTTP_OPTION_RECEIVE_TIMEOUT, which Microsoft documents as a PER-PACKET
   Winsock-layer read timeout ("applies to fetching each packet of data off
   the socket"). The wait for the response HEADERS is a separate option,
   WINHTTP_OPTION_RECEIVE_RESPONSE_TIMEOUT, which WinHttpSetTimeouts does not
   set and which defaults to 90 SECONDS. So a server that accepts, reads the
   request and then stalls could block the calling thread for a minute and a
   half no matter what the caller passed as timeout_ms -- precisely the
   "blocking an OBS thread indefinitely" failure that test exists to prevent.
   Now set explicitly, guarded by #ifdef so an older SDK still builds.

   That is a genuine defect on its own merits. Whether it is the whole
   explanation for the intermittency is NOT established: the same docs say
   this timeout "is checked only when data is received from the socket", so
   neither option guarantees a hard deadline -- that needs a watchdog calling
   WinHttpCloseHandle, deliberately not done here.

2. Evidence, so the next run says more than pass/fail. The probe now runs 5
   times and prints elapsed ms, ok, status, requests_seen and the backend's
   error string (carrying GetLastError) for every attempt, so one CI run
   yields a failure RATE and an error code. Each attempt gets a FRESH
   loopback server: the server handles one connection at a time on a single
   thread, so reusing it would leave attempts 2..n in the accept backlog --
   never accepted, a different scenario from the one that fails. Verified on
   Linux: 5/5 attempts give up at ~701ms.

Also: build.yml now has paths-ignore for **.md, LICENSE, NOTICE, and the two
release-only files. This is a full three-platform build behind a runner with
capacity:1, and six of them fired for one afternoon of documentation edits.
Nothing that feeds a build or a test is on that list. Tradeoff: a docs-only
push now shows no status at all rather than a green one.

The WinHTTP change cannot be compiled locally (Linux host); CI is its first
build. All 6 suites pass locally on Linux.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AzGnvQ6wfD7bw7PZN35ft9
jknapp added 1 commit 2026-09-10 01:38:00 +00:00
fix(http): enforce a hard deadline on WinHTTP by cancelling the request
Build / macOS (macos-latest) (push) Successful in 51s
Build / Linux (ubuntu-24.04) (push) Successful in 1m14s
Build / macOS (macos-latest) (pull_request) Successful in 48s
Build / Linux (ubuntu-24.04) (pull_request) Successful in 1m8s
Build / Windows (windows-latest) (push) Successful in 3m56s
Build / Windows (windows-latest) (pull_request) Successful in 3m56s
b23644fa3e
Setting WINHTTP_OPTION_RECEIVE_RESPONSE_TIMEOUT (previous commit) fixed the
original failure -- the request is now always cancelled instead of waiting out
a stall and returning 200 -- but the instrumented CI probe shows it is not
cancelled on TIME. Five attempts with a 700ms budget against a server that
accepts and then stalls 5s returned after 1490, 1529, 2485, 3493 and 4506ms,
every one of them ERROR_WINHTTP_TIMEOUT (12002). One exceeded the test's 4s
bound, which is why Windows CI was still red.

That is the documented behaviour, not a mystery: both receive timeouts are
"checked only when data is received from the socket", so an expired timeout is
not surfaced until the peer sends something. Neither option is a deadline.

It matters because `fetchSlots` is called synchronously on the OBS UI thread,
behind the properties dialog's "Refresh camera list" button
(obs-adapter/src/plugin-main.cpp:486, kPropertiesTimeoutMs = 5000). At the
overshoot ratio measured above, a stalling server freezes that dialog for
something like half a minute -- the exact failure the shortened timeout there
was chosen to avoid.

So: a watchdog thread that closes the request handle once the deadline passes,
which is the documented way to cancel a WinHTTP operation. `RequestDeadline`
owns the handle and both threads close it through an
`atomic::exchange(nullptr)`, so exactly one close ever happens. Failures are
reported as a timeout rather than as a raw GetLastError when the deadline is
what fired.

The ceiling is twice the caller's budget, not the budget itself: resolve,
connect, send and receive each get `timeout` from WinHttpSetTimeouts, so a
slow-but-progressing exchange can legitimately exceed one budget and must not
be cancelled. There is one accepted race, documented at the class: the caller
can load the handle just before the watchdog closes it, turning the call into
ERROR_INVALID_HANDLE instead. Both mean the deadline expired.

Cross-compiled with mingw-w64 (`-fsyntax-only`) rather than waiting on CI to
find syntax errors; also confirmed by preprocessor probe that
WINHTTP_OPTION_RECEIVE_RESPONSE_TIMEOUT is defined in those headers, so the
#ifdef guard is not silently skipping the option. Linux: all 6 suites pass.
Real verification is the Windows job's probe output.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AzGnvQ6wfD7bw7PZN35ft9
jknapp added 1 commit 2026-09-10 01:43:33 +00:00
test: assert the 2x-budget ceiling the watchdog now guarantees, not 4s
Build / macOS (macos-latest) (push) Successful in 53s
Build / Linux (ubuntu-24.04) (push) Successful in 1m23s
Build / macOS (macos-latest) (pull_request) Successful in 46s
Build / Linux (ubuntu-24.04) (pull_request) Successful in 1m19s
Build / Windows (windows-latest) (push) Successful in 3m57s
Build / Windows (windows-latest) (pull_request) Successful in 3m59s
17540c75b0
The Windows job went green, but CTest prints test output only on failure, so a
pass says nothing about WHAT cancelled the request. Under the old 4000ms bound
a pass is ambiguous: the watchdog firing at ~1400ms and WinHTTP's own erratic
cancellation (measured at 1490-4506ms for this same 700ms budget) both fit
under it.

So assert the guarantee the code actually makes now -- a hard ceiling of twice
the caller's budget -- at 2500ms, which is 1400ms plus slack for a loaded
runner. If the watchdog stops doing the work, roughly half the attempts land
above this and print their elapsed time and error string, instead of quietly
passing.

Linux is unaffected: curl honours the 700ms budget exactly, 5/5 at ~701ms.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AzGnvQ6wfD7bw7PZN35ft9
jknapp merged commit e763d61ed3 into main 2026-09-10 01:54:16 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: CyberCoveLLC/obs-streamer-tools-plugin#5