fix(java): make SDK worker threads daemon - #9925
Conversation
|
|
|
Thanks for the PR! Template looks good ✓ Problem: observed and documented, not theoretical — the linked report (#8835, Direction: aligned. The sibling daemon package already uses daemon threads for exactly this reason ( Size: small — 4 production lines (one per module) plus 36 lines of new tests. Not a core-infrastructure path, so the core-module gate doesn't apply. Approach: the scope is right — this is the exact one-line-per-module fix the issue suggests, and custom executor injection ( Risk: no elevated risk signals — none of the changed paths match the revert-correlated set. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测且有据可查,不是理论问题——关联报告(#8835, 方向:对齐。同仓库的 daemon 包出于同样原因已经使用 daemon 线程( 规模:很小——生产代码 4 行(每个模块各 1 行),新增测试 36 行。不属于核心基础设施路径,核心模块门槛不适用。 方案:范围合理——正是 issue 建议的每模块 1 行修法;自定义 executor 注入( 风险:无升级风险信号——改动路径均不在与 revert 相关的高风险清单中。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewReading only the issue first, my own proposal would have been exactly this: flip the daemon flag in both Things I verified against the code:
One pre-existing gap worth knowing (not introduced by this PR): the Test evidence — this PR's own CI, fetched via API (no PR code was executed in this review)The suite pins the change itself: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 A process-level "JVM actually exits" observation is not part of the suite — that consequence follows from the JVM's specified daemon-thread semantics once the flag is pinned by the tests above, and the in-flight daemon E2E job is the live-behaviour lane for this area. Real-scenario (tmux) testing: N/A — unattended CI run, and the change is SDK-internal with no TUI surface. 中文说明代码审查只看 issue 时,我自己的方案与此完全一致:把两个 对照代码核实过的点:
一个值得了解的既有缺口(非本 PR 引入): 测试证据——来自该 PR 自身 CI,通过 API 获取(本审查未执行任何 PR 代码)测试套件钉住了改动本身:不做该翻转, 进程级"JVM 确实退出"的观测不在套件之内——一旦上面的测试钉住 daemon 标志,该结果即由 JVM 规范保证的 daemon 线程语义得出;运行中的 daemon E2E 任务就是这一领域的运行期行为验证通道。真实场景(tmux)测试:不适用——本次为无人值守 CI 运行,且改动是 SDK 内部、无 TUI 界面。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal fix that does exactly what the linked issue prescribed; only non-blocking notes (the Stepping back: the problem is real and evidenced in-repo — the shipped examples need a forced exit to escape the pool threads. The fix is the smallest possible one: two one-line flips that match the sibling daemon package's existing convention, plus focused tests that fail without the change. The diff carries nothing unrelated, and the opt-out for custom executors is preserved. The Java matrix is already green on this commit, so I'm approving once the remaining CI lands green. Still in flight at review time: 中文说明置信度:4/5 —— 干净、最小化的修复,完全按关联 issue 给出的修法执行;只有非阻塞性备注( 整体来看:问题真实存在且仓库内有据可查——自带示例需要强制退出才能逃离线程池。修法是最小可能的修法:两行 1 行翻转,与同仓库 daemon 包既有惯例一致,并附带不做此改动就会失败聚焦测试。diff 没有夹带无关改动,自定义 executor 注入口也得以保留。该提交的 Java 矩阵已经全绿,因此待其余 CI 全绿后批准。 审查时仍在运行: — Qwen Code · qwen3.8-max Reviewed at |
doudouOUC
left a comment
There was a problem hiding this comment.
Review
Clean, minimal fix. The diff is exactly the two one-line flips that issue #8835 suggests, plus focused tests that fail without the change. No unrelated changes.
Verified
- Source files read: both
ThreadPoolConfigfactories are identical in structure, differing only in thread name prefix. ThesetDaemon(false)→setDaemon(true)change is mechanically correct in both. grepconfirms the only remainingsetDaemoncalls inpackages/sdk-javaafter this change areDaemonClient.daemonThreadFactory(already daemon) and the identical pair being flipped — no sibling pool keeps the JVM alive.MyConcurrentUtilsin both modules usesThreadPoolConfig.getExecutor()forCompletableFutureasync tasks. After the daemon change, these tasks get interrupted when the last non-daemon thread exits — this is the desired behavior and is exactly what the fix is meant to achieve.- The
clientmodule has ashutdown()method (callsshutdownNow()); theqwencodemodule does not. This asymmetry is pre-existing — before the fix, theqwencodemodule had no way to release its pool threads, which is the root cause of the reported hang.
Pre-existing (not introduced by this PR)
packages/sdk-java/client/.../AcpClient.java has a commented-out ThreadPoolConfig.shutdown() call. This is tech debt — the shutdown() method exists but is never called by its own module. After the daemon change, this doesn't affect JVM exit, but it means the client module never cleanly interrupts its in-flight pool tasks. Out of scope for this PR; worth a follow-up.
CI
All 5 Java SDK lanes pass: Java 11, 17, and 21 on ubuntu, Java 21 on macos and windows, plus Real daemon E2E / Java 11 and checkstyle. The single failing check is Test (ubuntu-latest, Node 22.x) — the Node.js unit test suite, completely unrelated to the Java SDK changes.
Verdict
No issues found. The fix is correct, scoped, and well-tested.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| Thread t = new Thread(r, "qwen_code_cli-pool-" + threadNumber.getAndIncrement()); | ||
| t.setDaemon(false); | ||
| t.setDaemon(true); |
There was a problem hiding this comment.
[Suggestion] This flip to daemon threads falsifies two user-facing doc pages that still document this exact pool as non-daemon: docs/developers/sdk-java.md:347 and packages/sdk-java/qwencode/README.md:348 both carry - **Daemon Threads**: false in their "Thread Pool Configuration" section, which otherwise matches this pool field-for-field (qwen_code_cli-pool-{number}, Core 30 / Max 100 / Keep-Alive 60s / Queue 300 / CallerRunsPolicy). An integrator reading either page will plan shutdown semantics around non-daemon pool threads — rely on the JVM staying alive until queued work drains, omit future.get()/shutdown discipline — and after this change the JVM exits as soon as their last non-daemon thread dies, silently dropping that work; others will add the System.exit() workarounds this PR exists to eliminate. Please update both entries to - **Daemon Threads**: true in the same PR (the two sections are copies of each other), ideally noting that tasks still running or queued in the default pool are abandoned when the JVM exits.
中文说明
改为 daemon 线程后,两个面向用户的文档页面仍记录着旧的默认值:docs/developers/sdk-java.md:347 与 packages/sdk-java/qwencode/README.md:348 的 "Thread Pool Configuration" 小节都写着 - **Daemon Threads**: false(该小节描述的正是这个线程池——qwen_code_cli-pool-{number}、Core 30 / Max 100 / Keep-Alive 60s / Queue 300 / CallerRunsPolicy 均与代码一致)。按文档集成的用户会基于非 daemon 线程规划退出语义——依赖 JVM 存活到排队任务跑完、省略 future.get()/shutdown 纪律——本改动落地后,JVM 会在其最后一个非 daemon 线程结束时直接退出,静默丢弃这些任务;也有用户会加上本 PR 正要消除的 System.exit() 变通写法。建议在同一 PR 中把两处都改为 - **Daemon Threads**: true(两处小节内容互为拷贝),最好并注明 JVM 退出时默认池中仍在运行/排队的任务会被放弃。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| Thread t = new Thread(r, "qwen_code_cli-pool-" + threadNumber.getAndIncrement()); | ||
| t.setDaemon(false); | ||
| t.setDaemon(true); |
There was a problem hiding this comment.
[Suggestion] The bundled customization example now actively contradicts this fix: ThreadPoolConfigurationExample.runCustomSupplierExample() (packages/sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/cli/example/ThreadPoolConfigurationExample.java:25) recommends ThreadPoolConfig.setExecutorSupplier(() -> (ThreadPoolExecutor) Executors.newFixedThreadPool(20)), and OpenJDK's Executors.DefaultThreadFactory.newThread actively forces non-daemon threads (if (t.isDaemon()) t.setDaemon(false);). Since getExecutor() returns the supplier's executor when one is set, anyone customizing per the official example gets a non-daemon pool and the JVM again refuses to exit after work completes — issue #8835 recurs, but only for users who customized, the hardest cohort to diagnose because the default pool now behaves correctly. A 3-arm probe (this module compiled from the reviewed commit) confirms it:
ProbeDefault (no supplier — this PR's default): pool thread isDaemon=true -> exit=0 (clean exit)
ProbeCustom (example's line verbatim): pool thread isDaemon=false -> killed by timeout, exit=124 (JVM hangs)
ProbeFixed (example with a daemon factory): pool thread isDaemon=true -> exit=0
Please update the example to a pool whose factory sets t.setDaemon(true) (mirroring the new default), or add a one-line comment warning that a custom supplier must create daemon threads to preserve JVM-exit behavior.
中文说明
自带的自定义示例现在与本修复直接冲突:ThreadPoolConfigurationExample.runCustomSupplierExample()(packages/sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/cli/example/ThreadPoolConfigurationExample.java:25)推荐 ThreadPoolConfig.setExecutorSupplier(() -> (ThreadPoolExecutor) Executors.newFixedThreadPool(20)),而 OpenJDK 的 Executors.DefaultThreadFactory.newThread 会强制创建非 daemon 线程(if (t.isDaemon()) t.setDaemon(false);)。由于设置 supplier 后 getExecutor() 返回的就是 supplier 的 executor,按官方示例自定义的用户会拿到非 daemon 线程池,JVM 再次无法退出——#8835 复现,且只发生在自定义过的用户身上,因默认池行为正常而最难排查。三组探针(用本评审 commit 编译该模块)证实了这一点:
ProbeDefault(无 supplier——本 PR 的默认值):池线程 isDaemon=true -> exit=0(正常退出)
ProbeCustom (示例中原样一行): 池线程 isDaemon=false -> 超时被杀,exit=124(JVM 挂起)
ProbeFixed (示例改用 daemon 工厂): 池线程 isDaemon=true -> exit=0
建议把示例改为使用设置了 t.setDaemon(true) 的线程工厂(与新默认值保持一致),或加一行注释,提醒自定义 supplier 必须创建 daemon 线程才能保持 JVM 可退出。
— qwen3.8-max via Qwen Code /review (v0.22.0)
a9d1da1 to
80622ef
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
Use daemon threads for the Java SDK default executors so background helper pools do not keep example or short-lived JVM processes alive after work completes. Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
80622ef to
d642e8a
Compare
|
Two-stage review report for PR #9925
Final verdict: no findings from completed round 1; round 2 inconclusive due to timeout. Recommend re-running round 2 at the current head if a second independent pass is still desired. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| Thread t = new Thread(r, "qwen_code_cli-pool-" + threadNumber.getAndIncrement()); | ||
| t.setDaemon(false); | ||
| t.setDaemon(true); | ||
| return t; |
There was a problem hiding this comment.
[Suggestion] R1-1: This flip to daemon threads falsifies two user-facing doc pages that still document this exact pool as non-daemon: docs/developers/sdk-java.md:347 and packages/sdk-java/qwencode/README.md:348 both carry - **Daemon Threads**: false in their "Thread Pool Configuration" section, which otherwise matches this pool field-for-field (qwen_code_cli-pool-{number}, Core 30 / Max 100 / Keep-Alive 60s / Queue 300 / CallerRunsPolicy). An integrator reading either page will plan shutdown semantics around non-daemon pool threads — rely on the JVM staying alive until queued work drains, omit future.get()/shutdown discipline — and after this change the JVM exits as soon as their last non-daemon thread dies, silently dropping that work; others will add the System.exit() workarounds this PR exists to eliminate. Please update both entries to - **Daemon Threads**: true in the same PR (the two sections are copies of each other), ideally noting that tasks still running or queued in the default pool are abandoned when the JVM exits. Still stands from round 1; both lines verified unchanged at d642e8ab.
中文说明
改为 daemon 线程后,两个面向用户的文档页面仍在把这个线程池记录为非 daemon:docs/developers/sdk-java.md:347 与 packages/sdk-java/qwencode/README.md:348 的 "Thread Pool Configuration" 小节都写着 - **Daemon Threads**: false(该小节描述的正是这个线程池——qwen_code_cli-pool-{number}、Core 30 / Max 100 / Keep-Alive 60s / Queue 300 / CallerRunsPolicy 均与代码一致)。按文档集成的用户会基于非 daemon 线程规划退出语义——依赖 JVM 存活到排队任务跑完、省略 future.get()/shutdown 纪律——本改动落地后,JVM 会在其最后一个非 daemon 线程结束时直接退出,静默丢弃这些任务;也有用户会加上本 PR 正要消除的 System.exit() 变通写法。建议在同一 PR 中把两处都改为 - **Daemon Threads**: true(两处小节内容互为拷贝),最好并注明 JVM 退出时默认池中仍在运行/排队的任务会被放弃。第 1 轮已提出,本轮复核确认在 d642e8ab 处两处仍未修改。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| Thread t = new Thread(r, "qwen_code_cli-pool-" + threadNumber.getAndIncrement()); | ||
| t.setDaemon(false); | ||
| t.setDaemon(true); | ||
| return t; |
There was a problem hiding this comment.
[Suggestion] R1-2: The bundled customization example actively contradicts this fix: ThreadPoolConfigurationExample.runCustomSupplierExample() (packages/sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/cli/example/ThreadPoolConfigurationExample.java:25) recommends ThreadPoolConfig.setExecutorSupplier(() -> (ThreadPoolExecutor) Executors.newFixedThreadPool(20)), and OpenJDK's Executors.DefaultThreadFactory.newThread actively forces non-daemon threads. Since getExecutor() returns the supplier's executor when one is set, anyone customizing per the official example gets a non-daemon pool and the JVM again refuses to exit after work completes — the linked issue's hang (java-sdk-threadpool-nondaemon-jvm-hang) recurs, but only for users who customized, the hardest cohort to diagnose because the default pool now behaves correctly. A probe against this commit's compiled module confirms it:
example's supplier verbatim: pool thread daemon=false -> work complete, main exiting -> exit=124 (killed by timeout — JVM hung)
same probe, daemon factory: pool thread daemon=true -> exit=0 (clean exit)
Note this is not a regression versus base — customized users hung identically before this PR; the fix covers the default path while the bundled example keeps steering users to the broken one. Please update the example to a pool whose factory sets t.setDaemon(true) (mirroring the new default), or add a one-line comment warning that a custom supplier must create daemon threads to preserve JVM-exit behavior. Still stands from round 1; the example is unchanged at d642e8ab.
中文说明
自带的自定义示例与本修复直接冲突:ThreadPoolConfigurationExample.runCustomSupplierExample()(packages/sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/cli/example/ThreadPoolConfigurationExample.java:25)推荐 ThreadPoolConfig.setExecutorSupplier(() -> (ThreadPoolExecutor) Executors.newFixedThreadPool(20)),而 OpenJDK 的 Executors.DefaultThreadFactory.newThread 会强制创建非 daemon 线程。由于设置 supplier 后 getExecutor() 返回的就是 supplier 的 executor,按官方示例自定义的用户会拿到非 daemon 线程池,JVM 再次无法退出——关联 issue 的挂起问题(java-sdk-threadpool-nondaemon-jvm-hang)复现,且只发生在自定义过的用户身上,因默认池行为正常而最难排查。用本评审 commit 编译该模块的探针证实:
示例中原样一行:池线程 daemon=false -> 任务跑完、main 退出 -> exit=124(超时被杀——JVM 挂起)
改用 daemon 工厂:池线程 daemon=true -> exit=0(正常退出)
注意这并非相对基线的回归——自定义用户在本 PR 之前同样挂起;本修复覆盖了默认路径,但自带示例仍在把用户引向坏路径。建议把示例改为使用设置了 t.setDaemon(true) 的线程工厂(与新默认值保持一致),或加一行注释,提醒自定义 supplier 必须创建 daemon 线程才能保持 JVM 可退出。第 1 轮已提出,本轮复核确认示例在 d642e8ab 处仍未修改。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| package com.alibaba.acp.sdk.utils; | ||
|
|
||
| import org.junit.jupiter.api.Test; |
There was a problem hiding this comment.
[Suggestion] This new test never executes: mvn test in packages/sdk-java/client resolves surefire 2.12.4 (no surefire >=3.x is declared in that pom), which collects zero JUnit 5 tests, and no CI workflow builds or tests the client module — so the client-side setDaemon(true) change ships with nothing gating it. Measured at this commit: mvn -B -f packages/sdk-java/client/pom.xml test reports Tests run: 0, Failures: 0, Errors: 0, Skipped: 0 with this test class absent from the run (the qwencode module, which pins surefire 3.5.4, ran 130 tests including its copy of this test), and the base-branch sdk-java.yml / release-sdk-java.yml workflows only cd into packages/sdk-java/qwencode. If a later change reverts the client-side flip back to setDaemon(false), every test command in the repository stays green and the non-daemon regression ships — the PR's stated unit-test guarantee holds only for qwencode. Declare maven-surefire-plugin >= 3.x in packages/sdk-java/client/pom.xml, mirroring qwencode (3.5.4 with <failIfNoTests>true</failIfNoTests>), so mvn test actually collects JUnit 5 tests instead of silently running zero; additionally consider adding the client module to the sdk-java.yml CI job.
中文说明
这个新增测试实际不会被执行:packages/sdk-java/client 的 mvn test 会解析到 surefire 2.12.4(该 pom 未声明 >=3.x 的 surefire),它收集不到任何 JUnit 5 测试;且没有任何 CI workflow 构建或测试 client 模块——因此 client 侧的 setDaemon(true) 改动在没有任何测试把关的情况下合入。在本评审 commit 上实测:mvn -B -f packages/sdk-java/client/pom.xml test 报告 Tests run: 0, Failures: 0, Errors: 0, Skipped: 0,本测试类不在运行列表中(而锁定 surefire 3.5.4 的 qwencode 模块跑了 130 个测试,包含它的同名测试副本);基线分支的 sdk-java.yml / release-sdk-java.yml 也只 cd 进 packages/sdk-java/qwencode。若后续改动把 client 侧翻回 setDaemon(false),仓库里所有测试命令仍是绿的,非 daemon 回归会直接合入——本 PR 声明的单测保证只对 qwencode 成立。建议在 packages/sdk-java/client/pom.xml 中声明 >=3.x 的 maven-surefire-plugin(对齐 qwencode 的 3.5.4 与 <failIfNoTests>true</failIfNoTests>),让 mvn test 真正收集 JUnit 5 测试、失败时报错而不是静默跑零个;也可考虑把 client 模块加入 sdk-java.yml 的 CI job。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| Thread t = new Thread(r, "acp-client-pool-" + threadNumber.getAndIncrement()); | ||
| t.setDaemon(false); | ||
| t.setDaemon(true); | ||
| return t; |
There was a problem hiding this comment.
[Suggestion] The daemon flip silently rewrites the shutdown contract of a public SDK executor, but nothing in the code records why daemon — the rationale lives only in the commit message. This module's only shutdown() call site is commented out (AcpClient.java:172), so this flag is what decides whether the pool can keep a JVM alive. A future maintainer reasoning that daemon threads drop in-flight work may revert the flag; with core-pool-size 30, no core-thread timeout, and no wired shutdown path, a single submitted task then pins the JVM open forever and reintroduces the exact hang this commit fixes — measured: the reverted state with one task exits only by timeout (exit=124), while this commit exits cleanly. Conversely, anyone adding a must-complete fire-and-forget task gets a silent drop at JVM exit — measured: with the pool saturated, a 31st queued task never ran and nothing was logged. Record the contract in the code:
| Thread t = new Thread(r, "acp-client-pool-" + threadNumber.getAndIncrement()); | |
| t.setDaemon(false); | |
| t.setDaemon(true); | |
| return t; | |
| Thread t = new Thread(r, "acp-client-pool-" + threadNumber.getAndIncrement()); | |
| // daemon: the pool must not keep short-lived JVMs alive; tasks still running at JVM exit are dropped by design | |
| t.setDaemon(true); | |
| return t; |
中文说明
这次 daemon 翻转静默改写了公开 SDK executor 的退出契约,但代码里没有任何地方记录"为什么用 daemon"——理由只存在于 commit message 中。本模块唯一的 shutdown() 调用点被注释掉了(AcpClient.java:172),因此这个标志位就是决定线程池能否让 JVM 存活的唯一开关。未来若有维护者认为 daemon 线程会丢弃在途任务而把它翻回去,在 core 池大小 30、无 core 线程超时、无接好的 shutdown 路径的情况下,只要提交过一个任务,JVM 就会永久挂起——本提交修复的挂起问题原样复发。实测:翻回后的状态提交一个任务只能等超时(exit=124),本提交则可正常退出。反过来,若有人往该池加入必须跑完的 fire-and-forget 任务,JVM 退出时会被静默丢弃——实测:池打满后第 31 个排队任务没有执行,也没有任何日志。建议把该契约写进代码:
(建议的修改见上方 suggestion 代码块:在 setDaemon(true) 处加一行注释说明 daemon 的契约。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
| Thread t = new Thread(r, "qwen_code_cli-pool-" + threadNumber.getAndIncrement()); | ||
| t.setDaemon(false); | ||
| t.setDaemon(true); | ||
| return t; |
There was a problem hiding this comment.
[Suggestion] Same undocumented daemon contract as the sibling client module, with an extra twist here: this module's ThreadPoolConfig has no shutdown() at all, so the daemon flag is the only lifecycle mechanism the module possesses — yet nothing in the file says so. The rationale lives only in the commit message; a future maintainer copying the client module's shape (or porting code between the two near-duplicate classes) may revert the flag or assume an equivalent shutdown path exists, and with no shutdown() here the only fix for a resulting exit-hang would be System.exit(). Measured at this commit: the reverted state with a single submitted task pins the JVM (exit=124 by timeout), and a queued 31st task under a saturated pool is silently dropped at exit (never ran, no log). Record the contract in the code:
| Thread t = new Thread(r, "qwen_code_cli-pool-" + threadNumber.getAndIncrement()); | |
| t.setDaemon(false); | |
| t.setDaemon(true); | |
| return t; | |
| Thread t = new Thread(r, "qwen_code_cli-pool-" + threadNumber.getAndIncrement()); | |
| // daemon: the pool must not keep short-lived JVMs alive; tasks still running at JVM exit are dropped by design | |
| t.setDaemon(true); | |
| return t; |
中文说明
与姊妹 client 模块相同的、未写入代码的 daemon 契约,这里还多一层:本模块的 ThreadPoolConfig 完全没有 shutdown(),daemon 标志位是本模块唯一拥有的生命周期机制——但文件里没有任何说明。理由只存在于 commit message 中;未来若有维护者照抄 client 模块的形态(或在两个近乎重复的类之间移植代码),可能翻回该标志位、或以为存在等价的 shutdown 路径,而本模块没有 shutdown(),届时退出挂起的唯一补救就只有 System.exit()。在本评审 commit 上实测:翻回后的状态提交一个任务即挂住 JVM(超时 exit=124);池打满后第 31 个排队任务在退出时被静默丢弃(未执行、无日志)。建议把该契约写进代码:
(建议的修改见上方 suggestion 代码块:在 setDaemon(true) 处加一行注释说明 daemon 的契约。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
What this PR does
Makes the Java SDK default executor threads daemon threads in both the
qwencodeandclientmodules. It also adds focused unit coverage that checks the default thread factories create daemon threads.Why it's needed
The Java SDK default pools currently create non-daemon worker threads and the
qwencodemodule does not expose a shutdown API. Short-lived examples or embedded SDK usage can therefore keep the JVM alive after work completes; the existing examples work around this with explicitSystem.exit(0). The daemon package already uses daemon threads for the same reason.Reviewer Test Plan
How to verify
Run these commands from the repository root or the relevant module directories:
Both tests should pass and assert that the default thread factories create daemon threads.
Evidence (Before & After)
N/A for UI. Static local checks:
git diff --checkpassed, including the new test files. Local Java test execution was not available because this machine has no Java runtime and nomvn; the GitHub Java SDK matrix should provide runtime verification.Tested on
Environment (optional)
Local machine lacks Java runtime and Maven:
java -versionreports no Java Runtime, andmvnis not installed.Risk & Scope
Linked Issues
References #8835 (
java-sdk-threadpool-nondaemon-jvm-hang).中文说明
这个 PR 做了什么
将 Java SDK 的
qwencode与client两个模块中的默认 executor 线程改为 daemon 线程,并新增聚焦单测,验证默认 thread factory 创建的是 daemon thread。为什么需要
当前 Java SDK 默认线程池创建的是非 daemon worker,且
qwencode模块没有暴露 shutdown API。短生命周期示例或嵌入式 SDK 使用完成后,JVM 可能因为后台线程池仍存活而无法退出;现有示例用显式System.exit(0)绕过了这个问题。daemon 包里同类线程已经使用 daemon thread。Reviewer Test Plan
如何验证
从仓库根目录或对应模块目录运行:
两个测试都应通过,并断言默认 thread factory 创建的是 daemon thread。
证据(Before & After)
非 UI 改动,N/A。本地静态检查:
git diff --check已通过,包含新增测试文件。本机缺少 Java runtime 和mvn,无法执行本地 Java 测试;运行时验证依赖 GitHub 的 Java SDK matrix。测试环境
环境(可选)
本机缺少 Java runtime 和 Maven:
java -version提示没有 Java Runtime,mvn未安装。风险和范围
关联 Issue
关联 #8835(
java-sdk-threadpool-nondaemon-jvm-hang)。