From b03af1bf26d720a83b6e0b10b24baddee1e4ab86 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Mon, 17 Aug 2026 22:53:09 +0000 Subject: [PATCH 1/2] docs(ENG-RELEASE-WINDOWS): spec the api-server gate's own ability to report #584 kills test_openai_api_server.exe with 0xC0000409 and prints nothing but the doctest version banner, so nothing about the failure can be read off either Windows job. One half of that is provable by inspection rather than inferred: the file holds joinable std::thread objects across assertions that throw, and ~thread on a joinable thread is std::terminate, which MSVC raises as the same status a /GS failure would. A named assertion failure therefore arrives as an opaque fail-fast with no reporter output. This spec lands before the conversion so the commit order shows it did. It is deliberately scoped as a REPORTING repair: it does not claim the cure, and #584 stays open when it lands. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode] --- .agents/specs/windows-test-thread-raii.md | 141 ++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 .agents/specs/windows-test-thread-raii.md diff --git a/.agents/specs/windows-test-thread-raii.md b/.agents/specs/windows-test-thread-raii.md new file mode 100644 index 000000000..be6c21172 --- /dev/null +++ b/.agents/specs/windows-test-thread-raii.md @@ -0,0 +1,141 @@ +# The Windows api-server gate becomes able to report its own failure + +Identity: `ENG-RELEASE-WINDOWS` + +Issue: [#584](https://github.com/mudler/vllm.cpp/issues/584) + +Parent specification: [windows-baseline-coverage.md](windows-baseline-coverage.md), +which narrowed #584 and listed it under `## Owed`. This spec takes the half that +document names as provable by inspection. + +Status: `ACTIVE`. Base `affc2a7fdfaa1a75c6c2b8bacd2e79b2990446f7`. + +## Scope + +`tests/vllm/entrypoints/openai/test_api_server.cpp` holds joinable +`std::thread` objects across assertions that throw. Convert every such site to a +scoped joiner, so that a failing assertion in that file is reported by doctest +instead of killing the process through `std::terminate`. + +In: that one test file. In: an exception barrier on each thread body, because an +exception escaping a thread function is the same `std::terminate` from the other +direction. Out: `src/`, `include/`, the CI workflow, the release script, and any +other test file — the same shape exists elsewhere and is recorded under `## Owed` +rather than fixed here. + +This is a **reporting** repair. It is not claimed as the cure for #584, and the +issue stays open when it lands. See `## What this does and does not establish`. + +## Platform anchors + +vLLM has no Windows lane and no equivalent test, so the anchors are language and +platform contracts rather than an upstream port: + +- `[thread.thread.destr]`: `~thread` calls `std::terminate` if the thread is + joinable. A `std::thread` member destroyed during stack unwinding therefore + ends the process. +- `[except.handle]/9`: an exception escaping the initial function of a thread + calls `std::terminate`. +- MSVC implements `abort()` — which `std::terminate` reaches through the default + handler — as `__fastfail(FAST_FAIL_FATAL_APP_EXIT)`. `__fastfail` raises + status `0xC0000409` for every fail-fast code and bypasses SEH by design, so + doctest's Windows handler never runs and its buffered `stdout` is discarded + unflushed. That is why the whole job output is the doctest version banner. +- `httplib::Server::stop()` (`third_party/httplib/httplib.h:11460`) is a no-op + while `is_running_` is false, and `listen_internal` sets that flag only after + it enters the accept loop (`:12027`). A joiner that does not account for this + can block forever on a server that has not started, which is why the scoped + server thread waits before it stops. + +## Design + +Two small types at the top of the test file, above the first `TEST_CASE`. + +`ScopedThread` owns one `std::thread`, runs the body inside `try`/`catch`, and +joins in its destructor. The caught exception is stored in an +`std::exception_ptr` and rethrown by the explicit `join()`, which is a +synchronisation point, so the store and the load do not race. The destructor +never rethrows, because a destructor that throws during unwinding is the failure +it exists to prevent. An optional stop action runs before the join so a body +that waits on something can be released. + +`ScopedServerThread` is `ScopedThread` for the case that dominates this file: it +serves an `ApiServer` and owns the `stop()` as well as the join. Its stop action +first waits, bounded, for `is_running()`, because `httplib::Server::stop()` does +nothing before the accept loop is up and a naive joiner would convert an +`0xC0000409` into a 180-minute CI timeout — a worse instrument, not a better one. +The bound is the same `500 x 2 ms` the call sites already used to wait for the +server, so a server that never starts costs one second and then joins. + +Owning the stop means the explicit `h.server.stop()` lines are removed at each +converted site. Calling `stop()` twice is not equivalent to calling it once: the +second call sees `is_running_` still true while the accept loop unwinds and +`svr_sock_` already exchanged to `INVALID_SOCKET`, which trips +`assert(svr_sock_ != INVALID_SOCKET)` at `httplib.h:11462` on every build that +is not `NDEBUG` — which is the Linux test build. One owner is the only shape +that is correct on both platforms. + +## Risks + +- **A joiner that hangs is worse than a fast-fail.** Handled above by the + bounded wait, and by keeping the stop action explicit for the two Windows + console-handler threads that block on an event. +- **The conversion is mechanical across many sites.** A missed `stop()` removal + would hang the Linux run rather than pass quietly, so the failure mode of a + mistake here is loud. +- **The Windows crash may not be a joinable-thread terminate.** Then the lane + stays red — but red with a name, which is the point. + +## Tests and evidence + +The instrument cannot assert its own effect on the platform where the effect +matters, because no Windows host is available to this row. What is gated: + +1. The file compiles and the suite runs green on Linux, with the doctest case + count unchanged before and after — the conversion adds no case and removes + none. +2. A red-first mutation, executed and recorded in `## Outcome`: make one + assertion inside a scoped-server case fail on purpose, and record what the + run prints. Before the change the expectation is process death; after it, a + named doctest failure with a `Status:` line. + +The doctest case-count assertion is explicit rather than "it passed", and the +count is asserted non-zero, because a `-tc` filter that matches nothing prints +`SUCCESS!`. + +## Gates + +- `scripts/agent-preflight.sh` +- `cmake --build --target test_openai_api_server` and the binary's own run +- `python3 scripts/check-commit-style.py`, `check-commit-trailers.py`, + `check-agent-record.py`, `check-issue-index-append-only.py`, `check-pr-size.py` + +## Stop conditions + +Stop and report if the build cannot complete in the free disk available; an +`ENOSPC` here makes unrelated checkers emit refusals that read as verdicts about +this diff. Stop rather than widen: converting the same shape in other test files +belongs to its own change. + +## What this does and does not establish + +Establishes: an assertion failure anywhere in this file is reported. Every +converted thread is joined on every path. + +Does not establish: that #584's fast-fail was a joinable-thread terminate. A +`/GS` cookie failure and a CRT invalid-parameter call raise the identical status +and are not excluded by anything in the log. The issue stays open, and this +change is what makes the next Windows run able to answer the question. + +## Owed + +- The same shape exists outside this file and is not converted here: + `tests/vllm/entrypoints/openai/test_conformance.cpp:418`, + `tests/vllm/v1/kv_offload/lmcache/test_lmcache_client.cpp:140,326` and + `tests/vllm/v1/kv_offload/lmcache/test_lmcache_connector.cpp:106` each join + under an `if (joinable)` in a destructor or teardown, which is the safe half, + but no other file was audited for a bare `std::thread` held across an + assertion. `test_lmcache_client` is one of the four executables the Windows + gate runs after this one, so it is next in line to be reached at all. Tracked + by [#584](https://github.com/mudler/vllm.cpp/issues/584) until this file's + repair lets the lane say what fails next. From 1f1b3027ccd2d9c0621237940987b8f847350a4b Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Mon, 17 Aug 2026 23:05:09 +0000 Subject: [PATCH 2/2] fix(ENG-RELEASE-WINDOWS): the api-server gate can report its own failure again Every socket case in test_api_server.cpp held a bare joinable std::thread across assertions that throw. A failing REQUIRE, or a bare json::parse on an unexpected body, unwound past the thread object, and ~thread on a joinable thread is std::terminate. Nothing caught an exception escaping serve() either, which is std::terminate from the other direction. On MSVC that reaches abort(), which is __fastfail, which raises 0xC0000409 and bypasses SEH. doctest's Windows handler never runs and its buffered stdout is discarded, so a NAMED assertion failure arrives as an opaque exit code with no Status: and no assertions: line -- which is all either Windows lane has printed since #584 opened. ScopedThread joins on every path, runs the body in a catch-all, and rethrows the escaped exception from join() where doctest can name it. ScopedServerThread adds the stop() for the shape that dominates the file, and waits for the accept loop before stopping, because httplib's stop() is a no-op until is_running_ is up and a joiner that ignored that would trade a 0.79 s fast-fail for a 180-minute CI timeout. It owns the stop exclusively, so the explicit h.server.stop() lines go: a second stop() is not a no-op, it trips assert(svr_sock_ != INVALID_SOCKET) on any build that is not NDEBUG. 17 sites converted, which is every std::thread in the file. The 6 client threads also moved below the vectors their bodies write into, so the join now happens before those vectors are destroyed. Measured, same build dir, CI's own no-NDEBUG configuration. Case count unchanged and non-zero: 62 cases / 733 assertions / SUCCESS on both arms. Red-first, with one assertion mutated to fail while the thread is joinable, each arm compiling rc 0 and showing one changed line: BEFORE exits 134 with "terminate called without an active exception" and "test case CRASHED: SIGABRT"; AFTER exits 1 with the named failure and no abort at all. This is a REPORTING repair, not the cure. A /GS cookie failure and a CRT invalid-parameter call raise the identical status and nothing in the job log distinguishes them, so #584 stays open and the next Windows run is what measures it. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode] --- .agents/specs/windows-test-thread-raii.md | 38 +++ .../entrypoints/openai/test_api_server.cpp | 217 ++++++++++++++---- 2 files changed, 204 insertions(+), 51 deletions(-) diff --git a/.agents/specs/windows-test-thread-raii.md b/.agents/specs/windows-test-thread-raii.md index be6c21172..f5817a878 100644 --- a/.agents/specs/windows-test-thread-raii.md +++ b/.agents/specs/windows-test-thread-raii.md @@ -103,6 +103,44 @@ The doctest case-count assertion is explicit rather than "it passed", and the count is asserted non-zero, because a `-tc` filter that matches nothing prints `SUCCESS!`. +## Measured + +Host `mudler-desktop`, Linux, GCC, `cmake -S . -B build-584 +-DVLLM_CPP_BUILD_TESTS=ON` — the CI `build-test-cpu` configuration, so `NDEBUG` +is NOT defined and `httplib`'s `assert` is live. Same build directory for every +arm below, target `test_openai_api_server`, `-j 4`. Base +`affc2a7fdfaa1a75c6c2b8bacd2e79b2990446f7`. Zero compiler warnings. + +**Case count, unchanged and non-zero.** Both arms report the same totals, so the +conversion added no case and removed none: + +| arm | run | +|---|---| +| before | `test cases: 62 \| 62 passed \| 0 failed \| 0 skipped`, `assertions: 733 \| 733 passed \| 0 failed`, `Status: SUCCESS!` | +| after | `test cases: 62 \| 62 passed \| 0 failed \| 0 skipped`, `assertions: 733 \| 733 passed \| 0 failed`, `Status: SUCCESS!` | + +**The mutation.** One assertion inside the socket-smoke case is made to fail +while the server thread is still joinable — `CHECK(res->status == 200)` on +`/health` becomes `REQUIRE(res->status == 999)`. `git diff --stat` confirmed one +changed line in each arm, and each arm compiled with rc 0, so neither reading is +a build failure wearing a pass. + +| arm | exit | what the run printed | +|---|---|---| +| before | **134** (`SIGABRT`) | `terminate called without an active exception`, then `test case CRASHED: SIGABRT` | +| after | **1** | the named failure and nothing else: `FATAL ERROR: REQUIRE( res->status == 999 ) is NOT correct! values: REQUIRE( 200 == 999 )` | + +`terminate called without an active exception` names the mechanism exactly: it +is `~thread` on a joinable thread, not an escaped exception. The `after` arm +exits 1 through the ordinary failure path with no abort at all. + +**Why this has been invisible on Linux.** The `before` arm still printed its +assertion, because `SIGABRT` is catchable and doctest's POSIX handler reports it +and flushes. MSVC's `__fastfail` is not catchable and bypasses SEH, so the same +`std::terminate` prints nothing there. The defect is platform-independent; only +its reportability is not, which is why a decade of green Linux runs is not +evidence against it. + ## Gates - `scripts/agent-preflight.sh` diff --git a/tests/vllm/entrypoints/openai/test_api_server.cpp b/tests/vllm/entrypoints/openai/test_api_server.cpp index 93a3f75bf..a965b3d37 100644 --- a/tests/vllm/entrypoints/openai/test_api_server.cpp +++ b/tests/vllm/entrypoints/openai/test_api_server.cpp @@ -22,12 +22,15 @@ #include #include #include +#include #include #include +#include #include #include #include #include +#include #include #if defined(_WIN32) @@ -115,6 +118,120 @@ using vt::DType; namespace { +// ─── Threads that can still report a failure (#584) ────────────────────────── +// +// Every socket case below runs the server on a background thread and asserts +// against it. A bare `std::thread` held across those assertions makes the file +// unable to report anything at all, through two separate std::terminate paths: +// +// 1. `~thread` on a JOINABLE thread calls std::terminate ([thread.thread.destr]). +// A failing REQUIRE, or a `json::parse` on an unexpected body, unwinds past +// the thread object and ends the process. +// 2. An exception escaping a thread's initial function is std::terminate too +// ([except.handle]/9), so a throw inside `serve()` does the same. +// +// On MSVC std::terminate reaches `abort()`, which is `__fastfail`, which raises +// status 0xC0000409 and bypasses SEH by design. doctest's Windows handler never +// runs and its buffered stdout is discarded, so a NAMED assertion failure +// arrives in CI as an opaque exit code with no `Status:` and no `assertions:` +// line — which is exactly what #584 has printed on both Windows lanes. +// +// These two types close both paths. They do not claim to fix whatever #584's +// fast-fail actually is; they make the run able to say so. + +// One thread, joined by the destructor on every path. The body runs inside a +// catch-all and the escaped exception is rethrown by `join()`, which is a +// synchronisation point, so the store and the load do not race. The DESTRUCTOR +// never rethrows: throwing while unwinding is the failure this type exists to +// prevent. `stop_request` runs before the join, for a body that waits on +// something and would otherwise never return. +class ScopedThread { + public: + template + explicit ScopedThread(Body&& body, std::function stop_request = {}) + : stop_request_(std::move(stop_request)), + escaped_(std::make_shared()), + thread_([slot = escaped_, fn = std::forward(body)]() mutable { + try { + fn(); + } catch (...) { + *slot = std::current_exception(); + } + }) {} + + ScopedThread(const ScopedThread&) = delete; + ScopedThread& operator=(const ScopedThread&) = delete; + // Movable so a vector of them can exist; the body captured the exception slot + // BY VALUE rather than capturing `this`, so a move leaves no dangling handle. + ScopedThread(ScopedThread&&) = default; + // Move ASSIGNMENT stays deleted: assigning onto a joinable thread is itself + // std::terminate, and nothing here needs it. + ScopedThread& operator=(ScopedThread&&) = delete; + + ~ScopedThread() { stop_and_join(); } + + // Join and surface an exception the body swallowed, at a point where doctest + // can translate and name it. + void join() { + stop_and_join(); + if (escaped_ && *escaped_) { + std::exception_ptr e = *escaped_; + *escaped_ = nullptr; + std::rethrow_exception(e); + } + } + + private: + void stop_and_join() noexcept { + if (!thread_.joinable()) return; + if (stop_request_) { + // A throwing stop action would defeat the whole point on the unwind path. + try { + stop_request_(); + } catch (...) { + } + } + thread_.join(); + } + + std::function stop_request_; + std::shared_ptr escaped_; + std::thread thread_; // declared last: constructed after the slot it reads +}; + +// `ScopedThread` for the shape that dominates this file — an ApiServer served on +// a background thread. It owns the `stop()` as well as the join, so a case that +// throws before its stop line is reached still ends. +// +// The stop action waits for the accept loop first. `httplib::Server::stop()` is +// a no-op while `is_running_` is false (`third_party/httplib/httplib.h:11460`), +// and `listen_internal` raises that flag only once it is in the loop (`:12027`), +// so stopping too early would leave the destructor blocked in `join()` forever — +// turning a fast-fail into a CI timeout, which is a worse instrument, not a +// better one. The bound is the same 500 x 2 ms the call sites already used. +// +// It owns the stop EXCLUSIVELY, and the call sites no longer call +// `h.server.stop()` themselves. A second `stop()` is not a no-op: it sees +// `is_running_` still true while the accept loop unwinds and `svr_sock_` already +// exchanged to INVALID_SOCKET, which trips `assert(svr_sock_ != INVALID_SOCKET)` +// at `httplib.h:11462` on every build that is not NDEBUG — which is this suite's +// own Linux build. +class ScopedServerThread { + public: + explicit ScopedServerThread(ApiServer& server) + : thread_([&server] { server.serve(); }, + [&server] { + for (int i = 0; i < 500 && !server.is_running(); ++i) + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + server.stop(); + }) {} + + void join() { thread_.join(); } + + private: + ScopedThread thread_; +}; + // ─── Synthetic weights (mirrors test_serving.cpp) ──────────────────────────── uint64_t Mix(uint64_t x) { x += 0x9E3779B97F4A7C15ULL; @@ -1240,7 +1357,7 @@ TEST_CASE("api_server: socket smoke — real HTTP requests over an ephemeral por const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); // Wait until the accept loop is up. for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); @@ -1299,8 +1416,7 @@ TEST_CASE("api_server: socket smoke — real HTTP requests over an ephemeral por CHECK(j.at("choices").at(0).at("message").at("role") == "assistant"); } - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins } // Route-registration gate over a real socket: /tokenizer_info is ABSENT (404) @@ -1319,7 +1435,7 @@ TEST_CASE("api_server: /tokenizer_info + /abort_requests are opt-in routes") { h.server.set_tokenizer(&Fixture(), kMaxModelLen); const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); @@ -1334,8 +1450,7 @@ TEST_CASE("api_server: /tokenizer_info + /abort_requests are opt-in routes") { REQUIRE(abort); CHECK(abort->status == 404); // no callback → route not registered - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins } SUBCASE("backings attached → routes serve (200)") { @@ -1350,7 +1465,7 @@ TEST_CASE("api_server: /tokenizer_info + /abort_requests are opt-in routes") { }); const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); @@ -1369,8 +1484,7 @@ TEST_CASE("api_server: /tokenizer_info + /abort_requests are opt-in routes") { CHECK(json::parse(abort->body).at("aborted") == 2); CHECK(aborted_calls == 1); - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins } } @@ -1389,15 +1503,14 @@ TEST_CASE("api_server: ConfigureUtilityEndpoints wires the production C8 surface auto with_server = [](ServerHarness& h, auto&& body) { const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); httplib::Client client("127.0.0.1", port); client.set_read_timeout(5, 0); body(client); - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins }; // RED: a default production server WITHOUT the wiring seam 404s every C8 route, @@ -1531,15 +1644,19 @@ TEST_CASE("api_server: concurrent requests share AsyncLLM without state races") const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); constexpr int kClients = 6; - std::vector clients; std::vector statuses(kClients, -1); std::vector texts(kClients); + // Declared AFTER the vectors its bodies write into, so the joining destructor + // runs BEFORE those vectors are destroyed. The previous order was safe only + // because a joinable `std::thread` ended the process instead of unwinding. + std::vector clients; + clients.reserve(kClients); for (int i = 0; i < kClients; ++i) { clients.emplace_back([&, i]() { httplib::Client client("127.0.0.1", port); @@ -1571,8 +1688,7 @@ TEST_CASE("api_server: concurrent requests share AsyncLLM without state races") for (int i = 1; i < kClients; ++i) CHECK(texts[static_cast(i)] == texts[0]); - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins } TEST_CASE("api_server: configured persistent-stream capacity remains readable") { @@ -1586,7 +1702,7 @@ TEST_CASE("api_server: configured persistent-stream capacity remains readable") kStreamCapacity + ApiServer::kControlWorkerHeadroom); const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); @@ -1618,8 +1734,7 @@ TEST_CASE("api_server: configured persistent-stream capacity remains readable") CHECK(response->status == 200); parked.clear(); - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins } TEST_CASE("api_server: stream capacity must be positive") { @@ -1659,7 +1774,7 @@ TEST_CASE( const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); @@ -1697,8 +1812,7 @@ TEST_CASE( CHECK(nodelay == 1); // RED until ApiServer calls set_tcp_nodelay(true) ::close(client_fd); - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins #endif // defined(__linux__) } @@ -2098,15 +2212,14 @@ TEST_CASE("api_server: the /v1/videos routes do not exist without a runner") { auto with_server = [](ServerHarness& h, auto&& body) { const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); httplib::Client client("127.0.0.1", port); client.set_read_timeout(5, 0); body(client); - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins }; SUBCASE("no runner: every video route 404s, and the core routes are unaffected") { @@ -2264,7 +2377,7 @@ TEST_CASE("api_server: transcriptions socket smoke (multipart), generate routes AsrHarness h; const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); @@ -2311,8 +2424,7 @@ TEST_CASE("api_server: transcriptions socket smoke (multipart), generate routes "parakeet-fixture"); } - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins } TEST_CASE("api_server: the audio routes do not exist on a TEXT server") { @@ -2330,7 +2442,7 @@ TEST_CASE("api_server: the audio routes do not exist on a TEXT server") { const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); @@ -2363,8 +2475,7 @@ TEST_CASE("api_server: the audio routes do not exist on a TEXT server") { CHECK(health->status == 200); } - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins } // ─── ARCH-ONE-SURFACE ROW 8: the server's --device seam ────────────────────── @@ -2518,7 +2629,7 @@ TEST_CASE("api_server: embeddings socket smoke; generate routes 404 on the " EmbedHarness h; const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); @@ -2554,8 +2665,7 @@ TEST_CASE("api_server: embeddings socket smoke; generate routes 404 on the " "llama-embed-fixture"); } - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins } TEST_CASE("api_server: /v1/embeddings does not exist on a TEXT server") { @@ -2571,7 +2681,7 @@ TEST_CASE("api_server: /v1/embeddings does not exist on a TEXT server") { const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); @@ -2586,8 +2696,7 @@ TEST_CASE("api_server: /v1/embeddings does not exist on a TEXT server") { CHECK(res->status == 404); } - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins } TEST_CASE("platform process: Windows command line preserves every argv byte") { @@ -2626,10 +2735,16 @@ TEST_CASE("platform shutdown: teardown drains an acquired console handler") { auto shutdown = std::make_unique( [&]() { ++stops; }); shutdown->SetBeforeDrainEventForTest(before_drain); - std::thread handler([&] { - CHECK(vllm::platform::ConsoleShutdown::DispatchControlEventForTest( - CTRL_BREAK_EVENT, acquired, resume)); - }); + // Both threads below block until `resume` is set, so their stop action is + // that SetEvent: an unwind must not park the joining destructor forever. + // `resume` is manual-reset, so setting an already-set event is a no-op and + // the explicit SetEvent calls further down stay exactly as they were. + ScopedThread handler( + [&] { + CHECK(vllm::platform::ConsoleShutdown::DispatchControlEventForTest( + CTRL_BREAK_EVENT, acquired, resume)); + }, + [&] { SetEvent(resume); }); const DWORD acquired_result = WaitForSingleObject(acquired, kWaitMs); if (acquired_result != WAIT_OBJECT_0) { SetEvent(resume); @@ -2640,10 +2755,12 @@ TEST_CASE("platform shutdown: teardown drains an acquired console handler") { CloseHandle(acquired); FAIL("console handler did not acquire state within timeout"); } - std::thread destroyer([&] { - shutdown.reset(); - destroyed.store(true, std::memory_order_release); - }); + ScopedThread destroyer( + [&] { + shutdown.reset(); + destroyed.store(true, std::memory_order_release); + }, + [&] { SetEvent(resume); }); const DWORD drain_result = WaitForSingleObject(before_drain, kWaitMs); if (drain_result != WAIT_OBJECT_0) { SetEvent(resume); @@ -3062,7 +3179,7 @@ TEST_CASE("api_server: /v1/audio/speech route registration is ADDITIVE over a re auto with_socket = [](ServerHarness& h, auto&& body) { const int port = h.server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&h]() { h.server.serve(); }); + ScopedServerThread server_thread(h.server); for (int i = 0; i < 500 && !h.server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(h.server.is_running()); @@ -3070,8 +3187,7 @@ TEST_CASE("api_server: /v1/audio/speech route registration is ADDITIVE over a re client.set_connection_timeout(5, 0); client.set_read_timeout(15, 0); body(client); - h.server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins }; SUBCASE("with NO speech family attached the route is 404 and nothing leaks") { @@ -3156,7 +3272,7 @@ TEST_CASE("api_server: a SPEECH-ONLY server serves speech and 404s the generate const int port = server.bind_to_any_port("127.0.0.1"); REQUIRE(port > 0); - std::thread server_thread([&server]() { server.serve(); }); + ScopedServerThread server_thread(server); for (int i = 0; i < 500 && !server.is_running(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(2)); REQUIRE(server.is_running()); @@ -3218,6 +3334,5 @@ TEST_CASE("api_server: a SPEECH-ONLY server serves speech and 404s the generate CHECK(json::parse(models_res->body).at("data").at(0).at("id") == "minimax-music3"); } - server.stop(); - server_thread.join(); + server_thread.join(); // stops the server, then joins }