fix(deepseek): release the PoW worker slot when spawning fails - #13097
diegosouzapw merged 1 commit into
Conversation
`solveInWorker` increments `activeWorkerCount` before constructing the Worker, but the `cleanup()` that decrements it lives inside the promise executor and only runs once the worker exists. Anything that throws first -- most obviously `resolveWorkerPath()` when the worker script is missing, since it resolves against `process.cwd()` -- leaves the counter permanently incremented. With `MAX_CONCURRENT_WORKERS = 2`, two such failures wedge the solver for the lifetime of the process: every later call rejects with "capacity reached (2)" while no worker is actually running, and the real cause is hidden. Construct the Worker inside a try/catch and release the slot before rejecting. Fixes diegosouzapw#13094
|
The checks shown here (Mergify, semgrep) are the ones that run without approval — the workflow runs for tests and typecheck are held in There are now 7 of these open, all small and independent, all from the same resource-leak audit:
Approving the held workflow runs (once per branch) is all that is needed to get real CI on them. No rush on review itself — I would just rather you judge them on this repo's CI than on my local runs. For what it is worth locally on Windows 11 / Node 24.19.0: Happy to rebase, split, or drop any of them if the batch is too much at once. |
…souzapw#13097) `solveInWorker` increments `activeWorkerCount` before constructing the Worker, but the `cleanup()` that decrements it lives inside the promise executor and only runs once the worker exists. Anything that throws first -- most obviously `resolveWorkerPath()` when the worker script is missing, since it resolves against `process.cwd()` -- leaves the counter permanently incremented. With `MAX_CONCURRENT_WORKERS = 2`, two such failures wedge the solver for the lifetime of the process: every later call rejects with "capacity reached (2)" while no worker is actually running, and the real cause is hidden. Construct the Worker inside a try/catch and release the slot before rejecting. Fixes diegosouzapw#13094
…agment (diegosouzapw#13200) Um caractere. O fragmento do diegosouzapw#13097 subiu sem o `- ` inicial e derrubou o `Merge integrity` para todo mundo que veio depois. Terceira ocorrência da mesma causa nesta release; a anterior foi o `reset-aware-model-family.md`, que o diegosouzapw#12711 consertou de carona.
Fixes #13094.
Problem
solveInWorkertakes the concurrency slot before the worker exists:resolveWorkerPath()resolves the worker script againstprocess.cwd()and throws when it isn't there. That throw escapes before any handler is wired up, so the counter is never decremented.MAX_CONCURRENT_WORKERSis 2, so two such failures disable the solver for the lifetime of the process. Every later call then rejects with:while no worker is actually running — and the real cause (missing worker script) is hidden behind a misleading capacity error.
Fix
Construct the Worker in a
try/catchand hand the slot back before rejecting, so the rejection carries the real error.Tests
tests/unit/deepseek-pow-slot-leak-13094.test.ts, two cases:worker script not found, notcapacity reached.The failure is triggered the same way users hit it —
process.chdir()into an empty temp dir soresolveWorkerPath()cannot find the script — rather than by mocking, and the cwd is restored infinally.Verified as a real detector: both fail on
release/v3.8.51before the change, with the first showingactual: 'DeepSeek PoW worker capacity reached (2)'. Both pass after.Scope
fail 0across the full DeepSeek PoW suite (11 tests, existing ones included), andtsc -p tsconfig.json --noEmitis clean.Same class of bug as the worker-slot leak in #12812, but on the logic counter rather than an OS thread, and reachable without any worker ever starting.