Replies: 2 comments
|
I reproduced this and published a discussion-first reference implementation: The change replaces one-shot fallback healing with a bounded per-link convergence loop for transient inspect, read, unlink, and create races. It accepts a same-target winner, while persistent real entries, an already-observed different target, and non-race filesystem failures still fail loud. Because Node exposes no compare-and-unlink primitive, the competing-target protection is explicitly best-effort over observed states rather than a cross-process lock. Verification on Windows:
No upstream PR was opened; this is a reviewable reference for the project’s Discussion-first workflow. |
|
Nice work — the convergence loop covers exactly the two crash sites we hit (readlink :380, unlink :381). Our original crash was macOS and your stress run was Windows, so I ran the macOS side (Node v25.4.0, your
Holds on the POSIX side. Loop closed. |
Uh oh!
There was an error while loading. Please reload this page.
Summary
When several
dshprocesses start concurrently while the profile module fallback needs healing (e.g. first launch after switching dsh versions), some processes crash with ENOENT insideensureSymlink. ThelstatSync → readlinkSync → unlinkSyncsequence has no concurrency guard; only the finalsymlinkSynccall tolerates losing the race (EEXIST branch).Environment
@deepseek-ai/dsh0.1.1-rc.2 (npmlatestas of 2026-08-24)$DSH_HOME=~/.dsh(default), profileheadlessObserved
Three
dsh --profile headless --patch <overlay> <prompt>processes launched concurrently right after switching from 0.1.0-rc.6 to 0.1.1-rc.2 (so every link in~/.dsh/profiles/node_modules/still pointed at the old install and needed re-pointing). Two of three crashed within ~270 ms, exit code 1:Re-running the same workload afterwards (fallback already healed) is stable — zero crashes across dozens of launches.
Root cause
ensureSymlink(packages/boot/app-boot/src/profile.ts:171, identical on master and in the 0.1.1-rc.2 bundle):lstatSync(line 174) is guarded — ENOENT is swallowed.readlinkSync(line 184) is unguarded — crash window: the link existed at lstat, another healer unlinked it before our readlink.unlinkSync(line 187) is unguarded — crash window: another healer already unlinked the wrong-target link.symlinkSync(line 190) is guarded (EEXIST + verify), and its comment (lines 192–194) shows concurrent healing was anticipated:The guard covers only the create step; the read and delete steps before it are missing the same "losing the race is success" semantics.
Why steady state is safe
When links already point at the right targets,
readlinkSync(link) === targetreturns early — no filesystem mutation, no race window. The bug only fires when many links need re-pointing AND launches are concurrent: version switch, first install, or a moved npx cache. That is why single-process usage never hits it.Independent reproduction
A controlled experiment with a synthetic
$DSH_HOMEpre-seeded with 120 wrong-target links: a single process healed all 120 without error; 16 concurrent processes reproduced 4 crashes (3× readlink ENOENT, 1× unlink ENOENT) on the first round.Suggested fix
Give
readlinkSync/unlinkSyncthe same race-tolerant semantics as the EEXIST branch: catch ENOENT, re-checkreadlinkSync(link) === target, treat a match as "another process already healed this link" and return. A lockfile aroundhealProfilesModuleFallbackwould also work but is heavier.Context
Found by a regression-eval harness (dsh-eval-harness) that runs concurrent headless dsh sessions and captures per-attempt stderr; the crash surface appeared exactly on the first run after the eval pinned dsh 0.1.0-rc.6 → 0.1.1-rc.2.
All reactions