diff --git a/.agents/specs/windows-test-thread-raii.md b/.agents/specs/windows-test-thread-raii.md new file mode 100644 index 000000000..f5817a878 --- /dev/null +++ b/.agents/specs/windows-test-thread-raii.md @@ -0,0 +1,179 @@ +# 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!`. + +## 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` +- `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. 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 }