fix(ENG-RELEASE-WINDOWS): the api-server gate can report its own failure again - #1159
Merged
Merged
Conversation
…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]
…ure 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]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue: #584 (stays OPEN — see the last section). Row
ENG-RELEASE-WINDOWS.Spec:
.agents/specs/windows-test-thread-raii.md.Both Windows lanes have printed nothing but the doctest version banner since
test_openai_api_server.exestarted running: noStatus:line, noassertions:line, exit-1073740791(0xC0000409). That is not extrainformation about the crash — it is the crash's own reporting failure, and
until it is closed nobody can read what fails.
0xC0000409is the status__fastfailraises for every fail-fast code, soabort()— and thereforestd::terminate()— reaches it, and__fastfailbypasses SEH by design, so doctest's Windows handler never runs and its buffered
stdoutis discarded unflushed.tests/vllm/entrypoints/openai/test_api_server.cppsupplies thatstd::terminate()at 17 places. Every socket case runs the server on abackground thread and then asserts against it, and a bare
std::threadheldacross those assertions ends the process by two separate paths:
~threadon a joinable thread callsstd::terminate([thread.thread.destr]).A failing
REQUIRE, or a barejson::parseon an unexpected body, unwindspast the thread object.
std::terminatetoo([except.handle]/9), so a throw inside
serve()does the same from the otherside. Nothing in the file caught that.
So a named assertion failure arrives in CI as an opaque exit code. This
change closes both paths for that file.
What changed
Two types at the top of the file.
ScopedThreadjoins in its destructor onevery path, runs the body inside a catch-all, and rethrows the escaped exception
from
join(), which is a synchronisation point — at a place where doctest cantranslate and name it. Its destructor never rethrows, because throwing while
unwinding is the failure it exists to prevent.
ScopedServerThreadis that for the shape which dominates the file: it serves anApiServerand owns thestop()as well as the join, so a case that throwsbefore reaching its stop line still ends.
Converted: 14 server threads, the 6-client vector in the concurrency case, and
the two console-handler threads in the
_WIN32-only teardown case — 17 in all,which is every
std::threadin the file. Every one of them holds an assertionthat can throw before its join.
Two details are load-bearing rather than incidental:
httplib::Server::stop()is a no-op whileis_running_is false(
third_party/httplib/httplib.h:11460) andlisten_internalraises that flagonly once it is inside the loop (
:12027). A joiner that ignored this wouldturn a 0.79 s fast-fail into a 180-minute CI timeout, which is a worse
instrument, not a better one.
h.server.stop()lines are removed, so the scoped type ownsthe stop exclusively. A second
stop()is not a no-op: it seesis_running_still true while the accept loop unwinds and
svr_sock_already exchanged toINVALID_SOCKET, which tripsassert(svr_sock_ != INVALID_SOCKET)athttplib.h:11462on every build that is notNDEBUG— which is this suite'sown Linux build.
The 6 client threads also moved below the two vectors their bodies write into,
so the joining destructor now runs before those vectors are destroyed. The old
order was safe only because a joinable
std::threadended the process insteadof unwinding.
Evidence
Host Linux, GCC,
cmake -S . -B build-584 -DVLLM_CPP_BUILD_TESTS=ON— the CIbuild-test-cpuconfiguration, soNDEBUGis NOT defined andhttplib'sassertis live. One build directory for every arm, targettest_openai_api_server,-j 4, zero compiler warnings. Baseaffc2a7fdfaa1a75c6c2b8bacd2e79b2990446f7.Case count, stated and non-zero rather than "it passed". Identical on both
arms, so the conversion added no case and removed none:
test cases: 62 | 62 passed | 0 failed | 0 skipped,assertions: 733 | 733 passed | 0 failed,Status: SUCCESS!test cases: 62 | 62 passed | 0 failed | 0 skipped,assertions: 733 | 733 passed | 0 failed,Status: SUCCESS!Red-first mutation. One assertion in the socket-smoke case is made to fail
while the server thread is still joinable:
CHECK(res->status == 200)on/healthbecomesREQUIRE(res->status == 999). Each arm compiled with rc 0 andgit diff --statshowed exactly one changed line, so neither reading is a buildfailure or an unapplied edit wearing a pass.
SIGABRT)terminate called without an active exception, thentest case CRASHED: SIGABRT - Abort (abnormal termination) signalFATAL ERROR: REQUIRE( res->status == 999 ) is NOT correct! values: REQUIRE( 200 == 999 )terminate called without an active exceptionnames the mechanism exactly: itis
~threadon a joinable thread, not an escaped exception.Why a decade of green Linux runs is not evidence against this. The
beforearm still printed its assertion, because
SIGABRTis catchable and doctest'sPOSIX handler reports it and flushes.
__fastfailis neither, so the samestd::terminateprints nothing on Windows. The defect is platform-independent;only its reportability is not.
Records:
scripts/agent-preflight.shrc 0,check-agent-record.pyagent record OK: ENGINE=157 MODEL=377 QUANT=82 KERNEL=51 BACKEND=85,check-issue-index-append-only.pyOK: issue index append-only,check-pr-size.pyOK: every explicit path class is within its review budget.,check-commit-style.pyandcheck-commit-trailers.pyOKover an explicittwo-commit range, and
agent-ready.pyAll gates green.What this does and does not establish
Establishes: an assertion failure anywhere in that file is now reported by name.
Does not establish that #584's fast-fail was a joinable-thread terminate. A
/GScookie failure and a CRT invalid-parameter call raise the identical statusand nothing in the job log distinguishes them, because no output survives to
say. No second cause was found by inspection of the
:1294-:1325windoweither: the
httplibassertinstop()is compiled out by the lane'sReleaseconfig, the worker pool is 12 threads and not an exhaustion story, andAsyncLLM's output handler already catches everything and is joined byshutdown().#584 therefore stays open. This is the reporting repair it needs first, and
the next Windows run is the measurement.
.agents/issue-index.mdis not appended to: #584 already has a row there(
ENG-RELEASE-WINDOWS), and the index forbids a second row for the same issue.The three places that must agree are that row, the spec, and this body.
FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode]