Repository navigation
Replies: 3 comments
|
Verified follow-up: source-level fix + regression tests + a reviewable branch Reporting back with a complete port since we hit this in a downstream DSH distribution (a self-contained bundle of the harness shard — the crash took down a user's whole session when a temp cleaner removed the spill dir mid-stream). The repo isn't accepting external PRs at this stage (CONTRIBUTING.md), so I've put the source-level port on a branch in a fork for side-by-side review, and everything is paste-able from the diff below. The fix — /** Open the spill file lazily and append `chunk` (and any prior chunks once). */
private spillAll(chunk: Buffer): void {
if (this.maxSpillBytes !== undefined && this.total > this.maxSpillBytes) {
this.discardSpill()
return
}
+ try {
- if (this.spillFd === undefined) {
- // Random suffix + O_EXCL + no-follow-equivalent ('wx' fails on any
- // existing path, symlink or not) + owner-only mode: defeats spill-path
- // prediction and symlink planting in shared tmp dirs.
- this.spillFile = join(
- this.spillDir,
- `dsh-subprocess-${process.pid}-${++spillCounter}-${randomBytes(6).toString('hex')}-${this.label}.log`,
- )
- this.spillFd = openSync(this.spillFile, 'wx', 0o600)
- for (const prior of this.chunks) writeSync(this.spillFd, prior)
- }
- writeSync(this.spillFd, chunk)
+ if (this.spillFd === undefined) {
+ // Random suffix + O_EXCL + no-follow-equivalent ('wx' fails on any
+ // existing path, symlink or not) + owner-only mode: defeats spill-path
+ // prediction and symlink planting in shared tmp dirs.
+ this.spillFile = join(
+ this.spillDir,
+ `dsh-subprocess-${process.pid}-${++spillCounter}-${randomBytes(6).toString('hex')}-${this.label}.log`,
+ )
+ this.spillFd = openSync(this.spillFile, 'wx', 0o600)
+ for (const prior of this.chunks) writeSync(this.spillFd, prior)
+ }
+ writeSync(this.spillFd, chunk)
+ } catch (error) {
+ // The spill dir is created once per process and assumed to persist;
+ // external temp cleaners (CCleaner, Storage Sense, cleanup scripts)
+ // violate that assumption mid-stream, and an uncaught ENOENT here
+ // escapes the stream 'data' handler and kills the whole harness.
+ // Degrade to the in-memory tail — the contract's truncated shape —
+ // and recreate the dir so later spills from any collector sharing
+ // this per-process dir succeed again.
+ this.discardSpill()
+ const code = (error as NodeJS.ErrnoException).code
+ if (code === 'ENOENT') {
+ try {
+ mkdirSync(this.spillDir, { recursive: true })
+ } catch {
+ // Unrecreatable dir: stay degraded; every in-memory read keeps working.
+ }
+ }
+ if (!OutputCollector.spillWarned) {
+ OutputCollector.spillWarned = true
+ console.warn(
+ `dsh-subprocess-local: output spill to ${this.spillDir} failed (${String(error)}); degraded to in-memory tail`,
+ )
+ }
+ }
}(plus Why: the collector's own tail-keep rationale — "errors and final results cluster at the end" — already defines the degraded shape, so an uncaught Regression tests (
The warn-once guard is a plain static so both branches cover with two collectors in one process; no Verification: the downstream distribution has shipped the equivalent guard against the bundled lib with a red/green suite — the unguarded copy crashes with ENOENT exactly like the original incident, the guarded copy degrades, recreates, and recovers. Ported to source here: all 10 Branch for review: Dewario/deepseek-harness@master...Dewario:spill-dir-deletion-guard (commit Happy to restate as a PR if you start accepting them — this discussion seemed like the right channel for now. Thanks for the great project! |
|
我在 恢复修复已有 Dewario 的 建议恢复时保留源码承诺的 验证环境为 macOS / Node 22.23.1。三项原版表征通过；补丁上 11 项通过、81 项无关测试跳过。权限表征当前断言实际的 可复跑的表征测试补丁（仅测试，不修改生产源码）--- /dev/null
+++ b/packages/subprocess/subprocess-local/tests/contribution-proof.spec.ts
@@ -0,0 +1,76 @@
+import { mkdtempSync, readFileSync, rmSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { afterAll, describe, expect, it } from 'vitest'
+import { OutputCollector } from '../src/spawn.ts'
+
+/** A unique, per-test spill directory so parallel workers never collide. */
+function freshSpillDir(): string {
+ return mkdtempSync(join(tmpdir(), 'dsh-contribution-proof-'))
+}
+
+const createdDirs: string[] = []
+
+afterAll(() => {
+ for (const dir of createdDirs) rmSync(dir, { recursive: true, force: true })
+})
+
+describe('issue #5902 — deleted spill directory before first overflow', () => {
+ it('throws ENOENT when the spill directory is gone before the first overflow (current bug)', () => {
+ const spillDir = freshSpillDir()
+ createdDirs.push(spillDir)
+
+ // A collecting stream that will spill: cap the in-memory tail to 4 bytes.
+ const collector = new OutputCollector(4, 100, 'stdout', spillDir)
+
+ // Simulate a temp cleaner / antivirus / manual sweep removing the
+ // private per-process spill directory mid-session.
+ rmSync(spillDir, { recursive: true, force: true })
+
+ // The first overflowing push must open the spill file inside the now-gone
+ // directory. `openSync(..., 'wx', 0o600)` fails with ENOENT and, because
+ // `spillAll` has no error handling, the exception escapes push().
+ expect(() => {
+ collector.push(Buffer.from('1234')) // fits exactly; no spill yet
+ collector.push(Buffer.from('5678')) // overflows → spillAll → openSync ENOENT
+ }).toThrowError(/ENOENT/)
+ })
+
+ it('throws ENOENT when the spill directory is deleted after a prior spill completed', () => {
+ const spillDir = freshSpillDir()
+ createdDirs.push(spillDir)
+
+ const collector = new OutputCollector(4, 100, 'stderr', spillDir)
+
+ // First collector spills successfully onto disk, proving the directory
+ // existed and the fd was opened.
+ collector.push(Buffer.from('aaaa'))
+ collector.push(Buffer.from('bbbb')) // triggers spill, opens spillFd
+ collector.seal()
+
+ // Now the directory is swept. A later, separate OutputCollector — still
+ // pointed at the same per-process spill directory — has never opened its
+ // own spill file, so its first overflow hits the same ENOENT.
+ const second = new OutputCollector(4, 100, 'stdout', spillDir)
+ rmSync(spillDir, { recursive: true, force: true })
+ expect(() => {
+ second.push(Buffer.from('wxyzlmno'))
+ }).toThrowError(/ENOENT/)
+ })
+
+ it('spills full output when the spill directory exists (control for the two above)', () => {
+ const spillDir = freshSpillDir()
+ createdDirs.push(spillDir)
+
+ const collector = new OutputCollector(4, 100, 'stdout', spillDir)
+ expect(() => {
+ collector.push(Buffer.from('aaaaaaaaaaaa'))
+ }).not.toThrow()
+
+ // Sanity: the spill actually landed and full output is recoverable.
+ const out = collector.finalize()
+ expect(out.truncated).toBe(true)
+ expect(out.spillPath).toBeDefined()
+ expect(readFileSync(out.spillPath!, 'utf8')).toBe('aaaaaaaaaaaa')
+ })
+})在 Dewario 补丁上运行的目录权限表征import { mkdtempSync, rmSync, statSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { expect, it } from 'vitest'
import { OutputCollector } from '../src/spawn.ts'
it('records the directory mode after Dewario recovery under umask 022', () => {
const dir = mkdtempSync(join(tmpdir(), 'dsh-recovery-mode-'))
const oldMask = process.umask(0o022)
const collector = new OutputCollector(4, 100, 'recovery', dir)
try {
expect(statSync(dir).mode & 0o777).toBe(0o700)
rmSync(dir, { recursive: true })
expect(() => collector.push(Buffer.from('abcdefgh'))).not.toThrow()
expect(collector.finalize()).toMatchObject({ text: 'efgh', truncated: true })
expect(statSync(dir).mode & 0o777).toBe(0o755)
} finally {
collector.finalize()
process.umask(oldMask)
rmSync(dir, { recursive: true, force: true })
}
}) |
|
Still reproducible on the installed latest, @deepseek-ai/dsh@0.1.5-rc.1 (Windows 11 25H2, Node v24.15.0) — same path, same uncaught ENOENT (spillAll openSync :766 → push :742 → Socket. :981). Deterministic via the internals.spillDir hook pointed at a missing directory plus a spill overflow (A/B: unpatched exit 1; with mkdirSync(spillDir, { recursive: true }) + try/catch, exit 0). So the guard just hasn't landed upstream on the latest channel yet. 补充：在已安装的 latest 版 @deepseek-ai/dsh@0.1.5-rc.1（Windows 11 25H2，Node v24.15.0）上仍可复现——同一代码路径、同样未捕获的 ENOENT（spillAll openSync :766 → push :742 → Socket. :981）。 用 internals.spillDir 指向不存在的目录 + 触发 spill 溢出即可确定性复现（A/B：未打补丁 exit 1，加 mkdirSync(spillDir, { recursive: true }) + try/catch 后 exit 0）。即该修复目前只是还没并进 latest 通道。 |
Uh oh!
There was an error while loading. Please reload this page.
Package:
@deepseek-ai/dsh0.1.2-rc.1 /@deepseek-ai/dsh-subprocess-local^0.1.2-rc.1 (line numbers below refer to the bundledlib/index.jsin the published package)Environment: Windows 11, Node v22.17.0
Severity: critical — a single deleted
%TEMP%directory terminates the entiredsh webserver and every live sessionSummary
OutputCollector.spillAll()indsh-subprocess-localcallsopenSync()/writeSync()with no error handling, synchronously inside a streamdatahandler. The collector's spill directory is created once per process (%TEMP%\dsh-subprocess-<random>, viamkdtempSync) and can never be recreated. When something deletes that directory mid-session — a temp cleaner, antivirus, or manual sweep — the first subprocess whose output exceeds the in-memory cap throws an uncaughtENOENTthat kills the wholedsh webprocess, not just the affected subprocess.Observed in the wild: a
dsh webinstance ~83 minutes into its run died exactly this way on its first-ever spill (the generated file name's spill counter was-1-).Stack (verbatim)
Root cause
lib/index.js:648-651— spill dir created once per process viamkdtempSync(join(tmpdir(), "dsh-subprocess-"))and cached indefaultSpillDir. The module imports nomkdir, so it can never be recreated.lib/index.js:710-721—spillAll()performsopenSync(this.spillFile, "wx", 384)andwriteSync(...)with no try/catch, synchronously inside the child stream'sdatahandler (push()at:693, wired at:862). Any failure escapes as an uncaught exception and terminates the entire server.@deepseek-ai/*packages ever deletes these dirs (a source-wide search finds only themkdtempline, the per-file name template, and dependency pins), so an external%TEMP%sweep of a live dir guarantees this crash on the next spill.dsh-subprocess-*dirs are never pruned — six accumulated over ten days of normal use on the reporting machine.Repro
npx @deepseek-ai/dsh web, open a session (the spill dir is created lazily when the first spill-capped subprocess spawns).%TEMP%\dsh-subprocess-*while the server runs.Suggested fix (applied and verified locally)
Wrap the spill path in try/catch; on failure degrade to the existing tail-only path via
discardSpill()(which already setsspillDisabled = true), recreate the dir onENOENTso later collectors can spill again, and warn once per process. AddmkdirSyncto thenode:fsimport.Optional hardening: prune stale
dsh-subprocess-*dirs at startup (they currently leak forever), and consider a process-leveluncaughtExceptionguard for the web server so one subsystem cannot terminate every session.All reactions