fix(sdk-java): let a terminal continuation start the next prompt - #7615
Conversation
A prompt reserves both a prompt slot and a stream-cleanup slot in
DaemonClient.submit, but releases them at different times: the prompt slot is
released synchronously in FutureTask.done(), immediately before the terminal
publication gate opens, while the stream-cleanup slot is only released once the
SSE stream has finished closing on the stream-close executor. The terminal path
in observe() closes that stream asynchronously and does not wait for it.
Both semaphores were sized to maximumConcurrentPrompts, so a caller chaining
prompts off completionFuture() at full capacity races the previous prompt's
close: when the close had not finished by the time the terminal was published,
startPrompt failed with DaemonClientCapacityException("Stream cleanup capacity
is exhausted"). This is how the documented chaining pattern behaves under load,
and it made terminalContinuationCanStartNextPromptAtClientCapacity flaky in CI.
Size the stream-cleanup semaphore to allow one draining cleanup per prompt
slot, which is exactly the overlap the release ordering can produce. Admission
backpressure is unchanged in kind: cleanups that stay stalled beyond that
headroom still fail fast, now after two generations instead of one.
The stream-close executor queue is derived from the same capacity, so it still
absorbs every reservation without rejecting work.
|
Thanks for the PR! Template: the headings differ from the repo template ( Problem: observed CI flake with a linked failure showing the exact exception chain ( Direction: straightforward concurrency fix in the Java daemon SDK. Clearly in scope, no direction concerns. Size: not applicable — changes are in Approach: the scope feels right. The fix is one production line — sizing Moving on to code review. 🔍 中文说明感谢贡献! 模板:标题结构与仓库模板不同(用了 问题:已观测到的 CI 间歇性失败,附有失败链接,异常链完整( 方向:Java daemon SDK 的并发修复,完全在范围内,无方向性顾虑。 规模:不适用——改动在 方案:范围合理。修复只有一行生产代码——将 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewIndependent proposal: given the root cause (prompt slot releases synchronously in Comparison with the diff: the PR does exactly this. No simpler path missed. Production change is one line in Test changes are correct:
Design doc updated in the same commit to describe the new capacity model. No scope creep, no unrelated changes. No critical blockers, no convention violations. TestingNo Java/Maven/Docker on this runner — cannot run The author also reports a causal check: injecting a 50 ms delay into 中文说明代码审查独立方案: 根据根因(prompt 槽位在 与 diff 对比: PR 完全按此方案实现,没有遗漏更简路径。 生产代码改动仅 测试改动正确:
设计文档在同一提交中更新,描述新的容量模型。无范围蔓延,无无关改动。 无关键阻塞项,无规范违反。 测试本 runner 无 Java/Maven/Docker——无法本地运行 作者还报告了因果验证:在 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is a textbook concurrency fix. The root cause is real and well-evidenced (linked CI failure, exact exception chain, clear analysis of the semaphore release-ordering mismatch). The fix is the minimal correct change — one production line sizing The independent proposal I wrote before reading the diff arrived at the same approach — I didn't find a simpler path the PR missed. CI is green on Java 11/17/21 across ubuntu, windows, and macos. The author's causal verification (inject delay → reproduces on main, passes with fix, reverting capacity change fails again) is exactly the kind of evidence that makes a concurrency fix trustworthy. 中文说明置信度: 5/5 —— 每个阶段都干净,毫不犹豫可以合并。 这是一个教科书式的并发修复。根因真实且有证据(CI 失败链接、完整异常链、对信号量释放顺序不匹配的清晰分析)。修复是最小正确改动——一行生产代码将 我在读 diff 之前写的独立方案得出了相同的方法——没有找到 PR 遗漏的更简路径。CI 在 Java 11/17/21、ubuntu/windows/macos 上全绿。作者的因果验证(注入延迟 → 在 main 上复现,修复后通过,回退容量改动又失败)正是让并发修复可信的那种证据。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Correct fix — doubling streamLifecycleCapacity to 2x prompt slots accounts for one draining cleanup per prompt slot. The Math.min guard prevents overflow. New test validates the fix, existing test updated for doubled capacity.
— qwen3.8-max-preview via Qwen Code /review
Problem
DaemonSessionClientTest.terminalContinuationCanStartNextPromptAtClientCapacityfails intermittently on CI (example):The flake exposes a real API defect rather than a bad assertion: chaining prompts off
completionFuture()while the client is at its prompt-concurrency limit can randomly fail.Root cause
DaemonClient.submitreserves two semaphores per prompt, but they are released at different times:promptSlotsis released synchronously inFutureTask.done(), right beforeafterCapacityReleaseopens the terminal publication gate.streamLifecycleSlotsis only released once the SSE stream has finished closing, becauseregisterStreamCleanupattaches awhenCompletecallback to the cleanup future, and the terminal path inobserve()closes the stream throughcloseStreamAsyncwithout waiting for it.So a prompt's stream-cleanup reservation outlives its prompt slot by one generation. Both semaphores were sized to
maximumConcurrentPrompts, which leaves no room for that overlap: when the previous prompt's close has not finished by the time its terminal is published, the continuation'sstartPromptfinds a free prompt slot but no free cleanup slot and throws. Normally the close wins the race; on a loaded runner it does not.Fix
Size the stream-cleanup semaphore to allow one draining cleanup per prompt slot — exactly the overlap the release ordering can produce. The stream-close executor queue is derived from the same capacity, so it still absorbs every reservation without rejecting work.
Admission backpressure is unchanged in kind: cleanups that stay stalled beyond that headroom still fail fast, now after two generations instead of one.
stalledStreamCleanupAppliesBackpressureBeforePromptExecutionis updated to stall two cleanups before asserting rejection, andpendingStreamCleanupDoesNotBlockNextPromptAdmissionis added to pin the other half of the contract: one pending cleanup must not block the next admission.With the fix,
terminalContinuationCanStartNextPromptAtClientCapacityno longer depends on timing at all — atmaximumConcurrentPrompts(1)the second prompt always finds the spare slot.Verification
mvn clean test— 108 tests, green, repeated 3×.mvn checkstyle:check— 0 violations.mvn -DskipTests package— green.closeStreamAsyncclose task reproduces the exact CI failure onmain(same exception chain, same line), and all three tests still pass with that delay once this fix is applied. Reverting only the capacity change with the delay still injected fails all three, so the new tests are not vacuous.中文说明
问题
DaemonSessionClientTest.terminalContinuationCanStartNextPromptAtClientCapacity在 CI 上间歇性失败(示例):这个 flake 暴露的是一个真实的 API 缺陷,而不是断言写得太严:客户端跑满 prompt 并发上限时,用
completionFuture()串接下一个 prompt 会随机失败。根因
DaemonClient.submit为每个 prompt 预留两个信号量,但两者释放时机不同:promptSlots在FutureTask.done()中同步释放,紧接着afterCapacityRelease就打开 terminal 发布 gate。streamLifecycleSlots要等 SSE 流真正关完才释放:registerStreamCleanup只是在 cleanup future 上挂了一个whenComplete回调,而observe()的 terminal 路径通过closeStreamAsync异步关流,并不等待。也就是说,一个 prompt 的 stream cleanup 预留会比它的 prompt 槽位多活一代。而两个信号量的容量都按
maximumConcurrentPrompts配置,没有给这次重叠留出空间:当上一个 prompt 的关流在其 terminal 发布时还没结束,续接里的startPrompt能拿到空闲的 prompt 槽位,却拿不到 cleanup 槽位,于是抛异常。平时关流总能赢下这场赛跑,机器负载高时就未必。修复
把 stream cleanup 信号量的容量放宽为每个 prompt 槽位允许一个正在排水的 cleanup —— 这正是释放顺序会产生的重叠量。stream-close 执行器的队列由同一个容量推导,因此仍能容纳所有预留而不会拒绝任务。
准入背压的性质没有变:滞留超出这个余量的 cleanup 依然快速失败,只是从滞留一代变成滞留两代。
stalledStreamCleanupAppliesBackpressureBeforePromptExecution相应改为挂住两个 cleanup 再断言拒绝;新增pendingStreamCleanupDoesNotBlockNextPromptAdmission钉住契约的另一半:单个未完成的 cleanup 不得阻塞下一次准入。修复后
terminalContinuationCanStartNextPromptAtClientCapacity完全不再依赖时序 —— 在maximumConcurrentPrompts(1)下,第二个 prompt 必然能拿到那个备用槽位。验证
mvn clean test—— 108 个测试全绿,重复跑 3 次。mvn checkstyle:check—— 0 violations;mvn -DskipTests package—— 通过。closeStreamAsync的关流任务中注入 50 ms 延迟,可在main上稳定复现 CI 的失败(异常链和行号完全一致);打上本修复后,带着这个延迟三个测试依然全过。若只回退容量改动、保留注入延迟,三个测试全挂,说明新增测试不是空断言。