server: remove two host side stalls in the decode loop at high concurrency

[danielhanchen]

Two host side costs in llama-server that stall the decode loop at high concurrency, found by
timing every phase of a step rather than by guessing. No backend, RPC or CUDA file is touched:
the change is measured on the CUDA backend and again on a -DGGML_CUDA=OFF -DGGML_RPC=OFF
build, and it does not depend on anything specific to this fork.

Where the host time goes

One DGX Spark, Qwen3.8-27B UD-Q4_K_XL, --parallel 32, 32 concurrent requests, 128 prompt and
256 generated tokens, greedy. Per decode iteration, milliseconds, and per call in
microseconds. This is host time inside the step, i.e. time in which the GPU has nothing to do.

span	before ms/iter	after ms/iter	before us/call	after us/call
post_decode (total)	13.004	7.000	13004	7000
common_sampler_sample	7.023	6.594	222.9	209.3
result path	5.739	0.257	182.1	8.1
send_partial_response	5.503	0.128	174.6	4.0
queue_results.send()	5.370	0.086	170.4	2.7
add_token	0.006	0.001	0.2	0.0
batch build (decode step)	0.013	0.011	12.9	11.0

queue_results.send() is 63 times cheaper per call and the whole result path 22 times cheaper.
The step's host stall drops from 13.0 ms to 7.0 ms, and what is left is almost entirely
common_sampler_sample, which is O(vocabulary) by construction and is not touched here.

The tracer's own summary of the same two cells, per step over 526 steps:

	step ms	build	submit	sync	post	sampling	send	GPU busy	idle, neither GPU busy
before	309.7	7.7	25.8	262.9	12.9	7.0	5.7	93.1%	21.3 ms
after	302.8	7.5	25.8	262.0	7.0	6.6	0.3	94.9%	15.3 ms

Whole cell throughput

Same node, tracing off, base and new bracketed base / new / base, one server load per arm, all
three concurrencies against each load. Aggregate tokens per second over the closed loop:

concurrency	base	new	base	new vs base bracket
1	11.63	11.45	11.44	inside the bracket, no change
8	53.24	53.22	53.42	inside the bracket, no change
32	93.31	98.99	98.02	+3.5% on the bracket mean, +1.0% on the nearer base

That is what the phase table predicts: 6.0 ms saved out of a 303 ms step is 2%, and there is
nothing to save at 1 or 8 slots because the result path is a per slot cost. The gain grows with
the slot count, because the old path was O(slots^2) per step.

The changes

1. server_response: deliver a result to the one thread waiting for it. Every pending
result lived in one vector behind one condition variable. A send walked the waiting id set
linearly, pushed, then notify_all woke every waiting HTTP thread, each of which re-took the
same mutex and scanned the whole vector before going back to sleep. At N slots that is N
wakeups and N scans per token, N^2 per decode step, all contending for the mutex the decode
thread needs to send the next token. Results are now queued on a per reader waiter: ids
registered together share one waiter, so a send is an O(1) lookup, a push and one wakeup. FIFO
order per reader is preserved, which is what scanning the shared vector from the front did.

It also fixes a leak that came out of testing this. remove_waiting_task_ids(), the plural
form, erased the ids from waiting_task_ids but, unlike the singular form, never purged
queue_results. Anything already queued for those ids stayed in the shared vector for the life
of the process, and was still returned by a later recv() naming one of them.
server_response_reader::stop() calls exactly that plural form, so a client that disconnects
with a result in flight leaves it behind. The per waiter queues cannot have the bug by
construction: the results live in the waiter and the waiter dies with its last id. Measured in
isolation over 200,000 reader lifecycles, each leaving one result queued at teardown, which is
what a disconnect during generation does (test-server-response leak 200000):

	RSS growth	per reader
before	11,000 kB	56 bytes, unbounded
after	68 kB	0.3 bytes, flat

2. Only keep per token probabilities when the request asked for them.
server_slot::generated_token_probs is read in exactly one place, send_final_response(), and
only under n_probs > 0. Every other request still pushed a completion_token_output per
token, each with a heap allocated string, into a list that grows for the whole generation and
is then discarded.

Correctness

Greedy, temperature 0, top_k 1, seed 42, cache_prompt false, eight requests (four
prompts, streamed and non streamed), one slot and one request in flight so the batch
composition is fixed, md5 of the concatenated output:

27B on CUDA,  base against base (control)  e2515bd5d500bc6aa47a695daa843b0c / e2515bd5d500bc6aa47a695daa843b0c
27B on CUDA,  base against this branch     e2515bd5d500bc6aa47a695daa843b0c / e2515bd5d500bc6aa47a695daa843b0c
CUDA=OFF RPC=OFF, master against master    000872d79ef3a4e2c1b736d3dacbbed6 / 000872d79ef3a4e2c1b736d3dacbbed6
CUDA=OFF RPC=OFF, master against branch    000872d79ef3a4e2c1b736d3dacbbed6 / 000872d79ef3a4e2c1b736d3dacbbed6

and with four slots and two of the requests streamed concurrently, which is the path the first
change touches, on the same CUDA=OFF RPC=OFF builds:

master  657e06f5c0d3cb5b30688771644b72db
branch  657e06f5c0d3cb5b30688771644b72db

The base against base control is there because with several requests decoded in one batch this
model is not run to run reproducible, so a concurrent harness cannot be used as a correctness
control: it produces two different md5s from the same binary.

KV cache and prompt cache behaviour is untouched. Nothing here goes near pre_decode(), the
common prefix match, the cache reuse path or llama_memory_*. Checked rather than asserted, on
a 29 token prompt with 8 slots and cache_prompt: true, reporting
tokens_evaluated / tokens_cached / cache_n:

probe	before	after
cold	29 / 36 / 0	29 / 36 / 0
warm, same prompt	29 / 36 / 28	29 / 36 / 28
warm, longer generation	29 / 44 / 28	29 / 44 / 28
shared prefix, different tail	25 / 32 / 21	25 / 32 / 21
cache_prompt: false	29 / 36 / 0	29 / 36 / 0
the same warm probe again after 90 s of client disconnect storm	29 / 36 / 28	29 / 36 / 28

Identical on every field, with identical content md5s, including after the storm.

generated_token_probs was checked the same way, since the second change gates it. Seven probe
shapes, all identical before and after: n_probs 10, 1 and 0, no n_probs, n_probs: 5 with a
stop word that fires so stop_type is word and the STOP_TYPE_WORD branch trims by
stop_word_toks.size() from the end, post_sampling_probs: true, and stream: true.

Under fault

tools/server/tests/test-server-response.cpp in this branch drives the public server_response
API directly: 15 assertions covering reader isolation, per id and bulk teardown, broadcast,
FIFO order, late registration, timeouts and concurrent churn. It reports one failure on the
parent of this branch, the leak above, and none here.

End to end, stories15M-q4_0.gguf with 8 slots and four wedge shapes injected at once: two raw
socket streaming clients with a 2 kB receive buffer that never read a byte and are held 25 s so
the HTTP writer blocks inside send(), two that read 256 bytes then close with
SO_LINGER {1, 0} (RST) while results are still being produced for their ids, one non streaming
request abandoned with an RST mid generation, and repeated /slots/{id}?action=erase while the
wedges are held. 24 healthy clients run through the storm and 8 more after it, with /health
polled at 2 Hz throughout.

	before	after
healthy requests completed	32 of 32	32 of 32
/health probes, all 200	50 of 50	50 of 50
distinct content md5s over the 4 prompts	4, identical set	4, identical set
server exit after the storm	clean, rc 0	clean, rc 0

That one does not flip and is not meant to: it is the control saying the storm changes nothing
and the output is byte identical under it.

RSS under sustained client disconnects, 1,200 streaming requests each killed after 400 ms with
exactly 1,201 slot releases on both arms. Neither a time based nor a per request comparison is
meaningful here, because a faster server generates more tokens inside the same 400 ms window;
normalised by tokens actually generated the two arms are indistinguishable:

	total tokens generated	RSS growth	per token
before	109,970	746,432 kB	6.79 kB
after	101,398	687,288 kB	6.78 kB

So the roughly 6.8 kB per generated token is a pre-existing leak that both arms share and that
is not the result queue: the isolated measurement above accounts for 56 bytes per reader of it
before and 0 after. It is out of scope here, most likely the prompt cache.

-DLLAMA_SANITIZE_THREAD=ON CPU builds of both arms, 120 s of load: 16 streaming clients killed
at random sub-second intervals, 8 healthy clients, /slots and /metrics polled at 10 Hz.
23 data races before, 22 after, and none of them in server_response or in the send, recv,
add or remove region of server-queue.cpp on either arm. Reported as the negative result it
is: TSan finds no race in the result queue before or after, so this is evidence that the rewrite
introduces none rather than a flip. All 45 reports are in server_context_impl::load_model() and
llm_graph_input_*::set_input / set_input_kq_mask_impl, reached through
server_queue::yield_to_queue(), and are pre-existing.

The full tools/server/tests pytest suite gives 361 passed on both arms, with the same six
failures on both, all of them -DLLAMA_OPENSSL=OFF in that build config. A fresh worktree has no
tools/server/tests/tmp and produces 11 failures and 9 errors unrelated to any change until it
is created.

Six fixes to one subsystem, found serially

The six commits below all touch server_response, and a reviewer seeing that should know they
were found one round at a time, not batched. The first three defects were found and fixed
together in cdb0ecb8e; fixing them exposed a fourth, fixing that exposed a fifth and sixth, and
so on through four further review rounds. Each fix narrowed the behaviour the next one was found
in, which is why the sequence is what it is:

commit	what it fixed	how it was found
cdb0ecb8e	sibling results returned to the wrong caller; a single timed receive missing a result; terminate() ignored on the absent-waiter branch	review round 1
2af4f9047	notify_one() waking the wrong subset receiver, a direct consequence of the id filtering added above; and the new test's fork() breaking the Windows build	round 2, the first item caused by the round 1 fix
4e865432b	a receive naming ids from two waiters parking on only one of them	round 3
031b480e5	arrival order not preserved across waiters; a reader not woken when its waiter is discarded	round 4
5488483eb	a partly registered receive parking on the one live id's condition	round 5

The honest reading is that the per-waiter rewrite was buggier than one round of review showed,
and that each round only became visible once the previous one was fixed. The last change is the
one that stops the sequence: sole_waiter() replaced the earlier "do these ids share a
waiter" predicate rather than gaining another case, and by returning a waiter only when it covers
every requested id it collapses "spread over several waiters" and "not fully registered" into a
single condition. Two of the six defects lived in the gap between those two ideas.

Every one of them was reproduced against the base commit before any code was changed, and the
reproductions are all in tools/server/tests/test-server-response.cpp.

Three regressions this PR introduced, and the commit that fixes them

The per waiter queues, as first written here, broke three things that the base commit got right.
All three were found in review, all three were reproduced against base before any code was
changed, and cdb0ecb8e fixes them. Recording them explicitly rather than quietly, because they
were introduced by this PR and not inherited.

case	base fbf9abcc7	this PR before cdb0ecb8e	after
a timed receive that starts before its ids are registered sees a result that arrives during the call	PASS, 200 ms	FAIL, waits the full 5 s and returns nullptr	PASS, 200 ms
terminate() is honoured while parked on ids that are not registered	PASS, caller terminates	FAIL, returns nullptr and the caller can loop forever	PASS
recv() returns only an id the caller asked for	PASS	FAIL, returns a sibling's result	PASS
the sibling's result is still delivered to its own reader	PASS	FAIL	PASS
latency for an id registered after the reader parked	10 ms	708 ms	10 ms

1. A shared waiter could hand a caller a sibling's result. Ids registered together share one
   waiter, and both receive paths popped the front of its queue without looking at the id, so
   recv({A}) for a reader that registered {A, B} could return B's result. take_result()
   now scans for a result whose id the caller asked for, oldest first, which is exactly what
   scanning the shared vector did. The front normally matches, and send(), the path the
   performance numbers above rest on, is untouched.

2. A single timed receive that started before its ids existed missed a result that arrived
   during the call. It waited on condition_gone, which nothing fired on registration, while
   a later send() only notified the newly created waiter's own condition. The caller slept out
   its whole timeout and reported a spurious nullptr. The old shared queue woke such a caller
   because every send notified the one condition; add_waiting_task_id() and
   add_waiting_task_ids() now notify condition_gone to restore that.

3. The absent-waiter branch never rechecked running, so terminate() was ignored there.
   recv_with_timeout() is now a single loop that checks running at the top and again after a
   timed-out wait on both branches, and takes one deadline for the whole call, so waiting for
   a registration and then for a result cannot add up to twice the timeout the caller asked for,
   which the earlier two-phase version could.

The test in this branch had been masking two of the three: it called recv_with_timeout() in a
retry loop, so it measured the 1 s poll instead of the missing wakeup and reported a pass at
708 ms. test-server-response now makes a single timed call, and forks a child for the
terminate() case so that "the caller terminates" can be asserted rather than assumed. That
fork is guarded with #ifndef _WIN32, so the target still builds where LLAMA_BUILD_TESTS
defaults on and Windows keeps 18 of the 19 assertions; I could not compile it there, so the
guard was checked by preprocessing the file both ways and confirming waitpid, WIFSIGNALED
and pid_t are absent under _WIN32 and the skip branch is present.

Filtering by id then exposed a fourth problem, fixed in 2af4f9047. Two threads taking disjoint
subsets of one add_waiting_task_ids() call share a waiter, so notify_one() could wake the
thread whose id has no result; it sleeps again while the thread whose result is queued waits out
its timeout. Measured at 2800 ms and a null result with four sibling receivers parked ahead of
the one being sent to, 0 ms after. send() and broadcast() now notify_all() on the waiter's
condition, which is one reader's own condition rather than the single global one the shared
vector used, so the common case of one thread per reader is still a single wakeup and the N^2
behaviour does not come back.

And a fifth, fixed in 4e865432b: ids registered by separate add_waiting_task_id() calls sit
in separate waiters, so no one waiter's condition covers a receive that names ids from both.
take_result() already scanned every waiter the ids map to, but the wait still parked on
whichever one the lookup returned, so a result for an id in the other waiter notified a
condition the reader was not on. Measured at 2700 ms and a null result when the send goes to the
id whose waiter was not picked, 0 ms after. Such a reader now parks on the shared condition and
send() notifies that too, guarded by a counter so the ordinary path pays one integer
compare and not a second notify: server_response_reader registers every id it wants in one
call, so that counter is zero for every reader in the tree.

Two more on the same path, fixed in 031b480e5. Results now carry an arrival sequence, because
iterating the requested id set otherwise picked an arbitrary waiter's queue and could return a
later result first, where the shared vector scanned from the front and could not. Each waiter's
queue is already in arrival order, so its first match is its oldest and only those per waiter
winners are compared; a receive naming ids from one waiter, which is every reader in the tree,
takes the same single scan as before. And remove_waiting_task_id() and
remove_waiting_task_ids() now notify the waiter they discard, because a reader that had
already selected it was left parked on a condition nothing would fire again once the id was
re-registered onto a new waiter. Measured at 2700 ms and a null result, 0 ms after, in both
cases.

One more, fixed in 5488483eb, and by replacing the predicate rather than adding a case to it.
The "do these ids share a waiter" test only looked at ids that are registered, so a receive
naming one live id and one that did not exist yet looked like a single waiter call and parked on
the live id's condition; registering the missing id put it on a different waiter and its send
notified only that one. sole_waiter() now returns a waiter only when it covers every
requested id, so an absent id keeps the reader on the shared condition until it appears, and
there is no longer a way to be "not spread over waiters" while still being incompletely covered.
The counter that decides whether send() notifies the shared condition is keyed on whether a
result could already be delivered to that reader, so the partly registered case falls out of the
same mechanism. Measured at 2700 ms and a null result, 0 ms after.

All of these were reproduced before anything was changed, and the reproductions are in
tools/server/tests/test-server-response.cpp, 25 assertions. Three of them needed driving in
both directions to fail at all, because which waiter the lookup reaches first depends on the
id set's iteration order, and a single direction passes while the defect is present.

None of it costs the ordinary path anything. server_response_reader registers every id it
wants in one add_waiting_task_ids() call before it ever receives, so sole_waiter() returns on
the first lookup, the arrival sequence comparison list is never built, and the counter that would
make send() do a second notify stays zero, leaving send() with one integer compare on top of
the O(1) lookup, push and wakeup.

Re-run against the fix: 18 of 18 assertions pass, test-server-queue passes, the isolated queue
retention is unchanged at 68 kB per 200,000 reader lifecycles, the concurrency storm still
completes 32 of 32 with an identical content md5 set, cache and probability behaviour are
identical to base field by field, pytest is unchanged, and ThreadSanitizer reports 21 warnings
against 22 before the fix and 23 on base, none of them in server_response or in the receive and
send region of server-queue.cpp on any arm.

Build coverage

-DGGML_CUDA=OFF -DGGML_RPC=OFF -DLLAMA_OPENSSL=OFF configures and builds clean, which is the
build the second md5 above was produced with. The CUDA build used for the tables is the
ordinary one.

What is deliberately not in here

The largest single host stall on this workload is not in the decode step at all: it is
create_checkpoint() during prefill. On the same cell it is 331 of the 392 ms that a prefill
iteration spends building its batch, 66 checkpoints of 149 MiB each for 32 prompts, 50.2 ms per
checkpoint, and the longest single batch build in the cell is 1.03 s, which is a direct time to
first token cost. Of those 50.2 ms, 43.7 are the std::vector<uint8_t>::resize() that zero
fills the buffer and only 6.5 are the state copy that follows.

Removing the zero fill looks obvious and is wrong: it made the copy 140 times slower, 6.5 ms to
917 ms per checkpoint, and cost 24% of whole cell throughput. The memset is doing real work, it
faults the destination pages in before the device to host copy touches them. The fix therefore
has to be reuse of an already resident buffer rather than removal of the memset, and that
carries a memory policy decision, so it is left out of this PR. For scale, running the same cell
with --ctx-checkpoints 0 moves median TTFT at 32 concurrent from 6239 ms to 3688 ms at
unchanged throughput.

[danielhanchen]

@codex security review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-09T11:47:33.401629Z	5488483	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 29fd150c7c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[danielhanchen]

What this changes, in four questions

1. Before the PR, what happened?

server_response held a single result vector behind a single condition variable. Every result woke every waiting HTTP thread, and each of them re-took the mutex and scanned the whole vector before going back to sleep. With N slots generating, that is N wakeups and N scans per token, so N^2 per decode step, all of it contending with the decode thread for the same mutex. Separately, every token copied a probability string and a vector into a list that grew for the whole generation and was then dropped, whether or not the request asked for per-token probabilities.

2. After the PR, what happens?

One waiter per reader, shared by every task id that reader registered in one call. A result is queued on the waiter that owns the id, so sending it wakes only the thread waiting for it, and that thread finds its result without searching. The probability buffer is only filled when n_probs > 0.

3. Is this a real issue or a fake one?

Real, and it gets worse exactly where it hurts. Run llama-server with --parallel above 1 and drive concurrent requests, which is the ordinary multi-user serving case. The wakeup storm scales with the square of the slot count and lands on the same mutex the decode loop needs, so the cost grows precisely as you add the concurrency the flag exists to provide. The per-token allocation is unconditional waste on every request that never asked for probabilities, which is nearly all of them.

4. If merged, does it break anything?

• Not RPC-gated and not Spark-specific: this is tools/server, live for every user of llama-server on one GPU, several GPUs or CPU only. Nothing here assumes a topology.
• Delivery order preserved. Results are held in a FIFO deque per waiter, so a reader still receives them in the order they were sent.
• The multi-id case is the one to get right, and it is handled explicitly: ids registered together share one waiter, so the first hit is the right one, and removing one id drops only that task's results while the waiter stays alive for its siblings.
• Termination still observed. The wait is bounded, so a terminate() landing after an id was removed from the map is still noticed, and a reader whose ids have left the waiting list is parked so it honours the timeout it asked for instead of spinning.
• Model agnostic, and no change to sampling, KV caching or prefix caching.

Verified: builds clean with RPC off; the server test suite passes. Note a fresh worktree has no tools/server/tests/tmp, which produces 11 spurious failures until it is created; that is a harness artefact, not this change.

Also carries a comment-reduction pass, AST-gated as comments-only against the pre-pass head.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 18abc68be5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[danielhanchen]

Simulation to destruction

Fault injected deliberately, never waited for. CPU only, no GPU at any point. This one needs three arms, not two, because the defect worth flipping was introduced and fixed inside this PR:

arm	commit	recv() when the waiter is absent
base	fbf9abcc7	scans a shared vector under one cv, waits forever on a condition that no longer fires for those ids
parent	29fd150c7	GGML_ASSERT(w && "recv() called for task ids that are not in the waiting list")
head	18abc68be	polls condition_gone in 1 s slices, parks that one caller

recv() (as opposed to recv_with_timeout()) has exactly one caller, tools/server/server-tools.cpp:2144, the next callback of the streaming tool/MCP endpoint, which runs on the HTTP thread. GGML_ASSERT is GGML_ABORT, so on the parent commit one dropped tool stream takes the process down for every other client.

New in this branch: tools/server/tests/test-server-response.cpp, 15 assertions plus a probe against the public server_response API, compiling unchanged on all three arms.

case	base	parent	head
send to an absent id dropped, live id still delivered	PASS	PASS	PASS
FIFO order per reader	PASS	PASS	PASS
two readers do not steal each other's results	PASS	PASS	PASS
a drained reader gets no extra result	PASS	PASS	PASS
removing one id keeps the sibling's result	PASS	PASS	PASS
the removed id's result is gone	PASS	PASS	PASS
bulk removal drops queued and late results	FAIL	PASS	PASS
broadcast delivers one copy per registered id	PASS	PASS	PASS
broadcast overrides the result id per target	PASS	PASS	PASS
an id registered after the reader parked is still served	PASS 10 ms	PASS 701 ms	PASS 708 ms
recv_with_timeout honours its timeout	PASS	PASS	PASS
concurrent send/recv/register/remove churn survives	PASS	PASS	PASS
recv() on dropped ids parks instead of aborting	PASS	ABORT rc=134	PASS
other readers unaffected by a parked reader	PASS	(never reached)	PASS
overall	1 failure	SIGABRT	0 failures

The parent aborts on tools/server/server-queue.cpp:454, and the test-server-queue.cpp already in this branch reproduces the same abort there in under 2.5 s.

A base defect this PR fixes that the description does not mention

server_response::remove_waiting_task_ids() (the plural form) on base erases the ids from waiting_task_ids but, unlike the singular form, never purges queue_results. Anything already queued for those ids stays in the shared vector for the life of the process and is still returned by a later recv() naming one of them. server_response_reader::stop() calls exactly that plural form. This branch cannot have the bug by construction, because the results live in the waiter and the waiter dies with its last id.

Measured in isolation, 200,000 reader lifecycles each leaving one result queued at teardown, which is what a client disconnect during generation does (test-server-response leak 200000):

arm	RSS growth	per reader
base fbf9abcc7	11,000 kB	56 bytes, unbounded
parent 29fd150c7	128 kB	0.7 bytes
head 18abc68be	68 kB	0.3 bytes, flat

End to end: many concurrent requests, four wedge shapes at once

stories15M-q4_0.gguf, 8 slots, injected simultaneously:

• W1 x2 a raw-socket streaming /completion with a 2 kB receive buffer that never reads a byte, held 25 s, so the server's HTTP writer blocks inside send()
• W2 x2 read 256 bytes then close with SO_LINGER {1, 0} (RST) while results are still being produced for those ids
• W3 a non-streaming request abandoned with an RST mid-generation
• W4 repeated /slots/{id}?action=erase while the wedges are held

24 healthy clients through the storm, 8 more after it, /health polled at 2 Hz throughout.

	base	head
healthy requests completed	32 of 32	32 of 32
/health probes, all 200	50 of 50	50 of 50
distinct content md5s over the 4 prompts	0298e0d0bcc0, 1268195d9a31, 1c2ac5d03e52, 701917a7aba5	identical set
server exit after the storm	CLEAN rc=0	CLEAN rc=0

This does not flip and is not supposed to: base does not abort here either, it just does N wakeups and N scans per token. It is the control that says the storm changes nothing and the output is byte identical under it.

ThreadSanitizer

-DLLAMA_SANITIZE_THREAD=ON CPU builds of base and head, run on the second node with setarch -R (TSan cannot map its shadow with ASLR on this kernel), 120 s of load: 16 streaming clients killed at random sub-second intervals, 8 healthy clients, /slots and /metrics polled at 10 Hz.

	base	head
WARNING: ThreadSanitizer: data race	23	22
of those, any frame in server_response::	0	0
of those, any frame in the send/recv/add/remove region of server-queue.cpp	0	0

I am reporting this as the negative result it is. All 23 base and all 22 head reports are in server_context_impl::load_model() and llm_graph_input_*::set_input / set_input_kq_mask_impl, reached through server_queue::yield_to_queue(), and are present on master. TSan finds no race in the result queue on either arm, so it is not a flip pair here; it is evidence that the rewrite introduces none. The 23 pre-existing races in the model load and graph input paths are a separate finding about master and I will raise them separately. The unit harness under TSan: 0 warnings on both arms, same PASS/FAIL pattern as the release build.

KV cache and prefix cache, checked rather than assumed

29 token prompt, 8 slots, cache_prompt: true:

probe	base tokens_evaluated / tokens_cached / cache_n	head	content md5
cold	29 / 36 / 0	29 / 36 / 0	identical
warm, same prompt	29 / 36 / 28	29 / 36 / 28	identical
warm, longer generation	29 / 44 / 28	29 / 44 / 28	identical
shared prefix, different tail	25 / 32 / 21	25 / 32 / 21	n/a
cache_prompt: false	29 / 36 / 0	29 / 36 / 0	n/a
the same warm probe again after 90 s of disconnect storm	29 / 36 / 28	29 / 36 / 28	identical

Identical on every field, including after the storm.

generated_token_probs gating

generated_token_probs has exactly one reader, send_final_response() at server-context.cpp:2032, inside if (n_probs > 0), including the STOP_TYPE_WORD branch that trims by stop_word_toks.size() from the end, which is the case most at risk from the add_token() early return. Seven probe shapes:

probe	probability entries	base vs head
n_probs: 10	24	identical
n_probs: 1	24	identical
n_probs: 0	0	identical
no n_probs	0	identical
n_probs: 5 with a stop word that fires (stop_type = word)	1	identical
n_probs: 5, post_sampling_probs: true	24	identical
n_probs: 5, stream: true		identical

Memory at equal request count

A time-based comparison is misleading because this branch serves more requests in the same wall time. 1,200 streaming requests each killed after 400 ms, 8 at a time, exactly 1,201 slot releases on both arms:

arm	RSS start	RSS after	growth
base	98,148 kB	844,580 kB	746,432 kB
head	99,272 kB	786,560 kB	707,288 kB

5% lower at equal work. The ~600 kB per request growth itself is present on both arms and is not the result queue (the isolated measurement above accounts for 56 bytes of it on base and 0 on head); it is a pre-existing leak elsewhere, most likely the prompt cache, and is outside this PR.

A latent API sharp edge, reported not blocking

recv() and recv_with_timeout() pop w->results.front() without checking the id is in id_tasks. Ids registered together share one waiter, so recv({A}) when the reader registered {A, B} returns B's result. The probe in the new test shows it:

base    PROBE recv() with a subset of a shared waiter  returns nullptr (id filtered)
parent  PROBE recv() with a subset of a shared waiter  RETURNS THE SIBLING'S RESULT (id=201)
head    PROBE recv() with a subset of a shared waiter  RETURNS THE SIBLING'S RESULT (id=201)

Unreachable today: server_response_reader::next() always passes the whole set, and post_task() / post_tasks() both assert id_tasks.empty() so a reader can only register once. Worth one line of comment on recv() at most.

Related and also benign: an id registered after a reader has parked takes up to 1 s longer to be noticed here (708 ms) than on base (10 ms), because the parked reader polls condition_gone rather than being woken by send(). Every caller in the tree registers its ids before posting the task and before calling recv, so the window does not occur.

Regression suite

tools/server/tests pytest, -k "not slow", curl-enabled builds: 6 failed, 361 passed, 6 skipped on base and identically here. The same 6 failures on both, all of them -DLLAMA_OPENSSL=OFF in my config. Note for anyone else running it: a fresh worktree has no tools/server/tests/tmp and you get 11 failures and 9 errors unrelated to any change until you mkdir it.

Answers to the four questions

• What happened before. On base every send() woke every waiting HTTP thread, each of which re-took the same mutex and scanned the whole shared vector: N wakeups and N scans per token, N^2 per decode step, contending for the mutex the decode thread needs. Base also leaks any result still queued for a reader torn down through remove_waiting_task_ids(). On the parent commit, a recv() for ids that had already left the waiting list was a GGML_ABORT on the HTTP thread.
• What happens after. A send is a map lookup, a push and one wakeup; a reader whose ids have gone parks itself and nothing else; results die with their waiter.
• Real or fake. The abort is real and reproduced deliberately, twice, on the parent commit. The leak is real and quantified. The N^2 wakeup cost is real but is a performance claim and is the description's own evidence, not something I set out to re-measure.
• Does merging break anything. Nothing found. 32 of 32 concurrent requests complete through four simultaneous wedge shapes with byte identical output, /health never fails, the server exits cleanly, prefix and KV cache figures are identical field by field including after a 90 s disconnect storm, all seven probability shapes are identical, RSS at equal request count is 5% lower, the pytest suite is identical, and ThreadSanitizer finds no race in the changed paths on either arm.

What I could not run

Windows and macOS: no machine for either. From source, the changed code is std::unordered_map, std::shared_ptr, std::deque, std::mutex and std::condition_variable, with no platform-specific threading primitives and no pthread calls, so the only portability question is whether condition_variable::wait_for is monotonic-clock based; libstdc++, libc++ and MSVC's STL all use a steady clock for wait_for (as opposed to wait_until with a system clock), so the 1 s poll cannot be skewed by a wall-clock change on any of them. x86-64 was not run either; nothing here is architecture dependent.

[danielhanchen]

Correction to the last paragraph of my previous comment. I asserted from memory that condition_variable::wait_for is steady-clock based on libstdc++, libc++ and MSVC's STL. That is what the standard requires (wait_for(lock, rel_time) is specified as wait_until(lock, steady_clock::now() + rel_time)), but it has not always been what the implementations did, and the accurate version matters here because both recv() and recv_with_timeout() now depend on a wait_for deadline rather than only on a notification.

On pthread platforms condition_variable was historically implemented with pthread_cond_timedwait(), which waits on CLOCK_REALTIME, so a steady_clock deadline was converted back to the system clock and a wall-clock warp could cut a wait short or extend it. Both libstdc++ and libc++ now specialise the steady_clock case onto pthread_cond_clockwait() with CLOCK_MONOTONIC where glibc provides it (llvm D65339, and the background is written up in Mike Crowe, "It's About Time", ACCU Overload 156). libc++ additionally does not report cv_status::timeout unless the caller's own clock has actually reached the deadline. MSVC's STL does not use pthreads at all; it builds on Win32 condition variables with relative timeouts, so the issue does not arise there.

Practical effect on this change: on an old glibc without pthread_cond_clockwait, a system clock step backwards could make the 1 s condition_gone poll and the recv_with_timeout deadline wait longer than asked. That is a pre-existing property of every wait_for in the tree, it degrades to "waits longer" rather than "misses a result", and it is bounded by the next notification. Not a blocker, but I should not have stated it as flatly as I did, and I could not test it: I have one glibc version here and did not step the system clock.

[danielhanchen]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 5488483eb2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".