Skip to content

Commit 5023630

Browse files
os-litantclaude
andauthored
fix(cli): keep the published binary's stderr off the blocking write path (#15496)
`bin/run.js` now installs `keepStderrNonBlocking()` before oclif can write a byte, and the guard compiles from `src/` into `dist/` so a published install actually carries it. Measured on the built binary: `os dev --verbose` piped to a reader that stops draining parks the main thread in `write(2)` 3.1 s later, 4 of 4 runs, fd 2, `O_NONBLOCK=false`, `wchan=sock_alloc_send_pskb` — parked 28.9 s, ignoring SIGINT, released only by the reader resuming. The clearing that persisted came from a grandchild (the esbuild service, inherited stderr), so the re-assert has to sit on the write path rather than run once at startup. Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N Co-authored-by: Claude <noreply@anthropic.com>
1 parent a104ad2 commit 5023630

9 files changed

Lines changed: 624 additions & 123 deletions
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
The published `os` binary no longer freezes in the kernel when whatever is reading its output stops draining.
6+
7+
Node puts the CLI's stderr on the non-blocking write path when it opens the pipe, so a write to a reader that has stopped is buffered rather than parking the thread. libuv clears that flag again in the pre-exec of every child spawned with **inherited** stdio — and inheriting is `dup2`, so the flag lives on an open file description the spawner shares. Clearing it for the child clears it for the CLI too.
8+
9+
Measured on the built binary, `os dev --verbose` with its output piped to a reader that stopped draining: `os dev` spawns `os serve --dev` with inherited stdio at 2.8 s, that child spawns the esbuild service with inherited stderr at 5.2 s, and fd 2 stays blocking for the rest of the run. 3.1 s after the reader stopped, the main thread sat in `write(2)` (`wchan=sock_alloc_send_pskb`), 4 of 4 runs — parked 28.9 s, **ignoring SIGINT while parked**, and released only when the consumer resumed. Not a crash and not a timeout: alive, idle, unresponsive, with an empty log. Anything that pipes `os dev` and reads it slowly — a CI log collector, a backgrounded runner, a supervisor that stops draining while it does work — could park the CLI this way.
10+
11+
`bin/run.js` now installs `keepStderrNonBlocking()` before oclif can write a byte. The guard re-asserts `O_NONBLOCK` immediately ahead of each write, which is what the measurement requires: the clearing that persisted was made by a **grandchild** the CLI does not spawn and cannot see, so a one-shot at startup would be undone silently and no change to the CLI's own spawn sites would have prevented it.
12+
13+
The guard itself is not new — it shipped in no published install. It lived at `packages/cli/bin/stderr-nonblocking.mjs`, and `files` names only `dist`, `README.md` and `CHANGELOG.md`; npm packs a `bin` **target** regardless of `files`, which is why `bin/run.js` reached every install and the module beside it reached none. It now compiles from `src/utils/stderr-nonblocking.ts` into `dist/`, under the whitelist that was already there.
14+
15+
Nothing about which arguments the CLI accepts, what it prints, or what it exits with changes. The refusal of `setBlocking(true)` in `src/utils/format.ts` stands and is untouched — this is its inverse, and what keeps its premise true.

‎packages/cli/bin/run-dev.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
// exactly as it did.
1414
import { flush, handle, run, settings } from '@oclif/core';
1515

16-
import { keepStderrNonBlocking } from './stderr-nonblocking.mjs';
16+
import { keepStderrNonBlocking } from '../src/utils/stderr-nonblocking.ts';
1717

1818
/**
1919
* How long stderr may make NO PROGRESS before this shim stops waiting for it.
@@ -189,7 +189,7 @@ settings.debug = true;
189189
// kernel with the diagnostic still unwritten, and only a reader or a kill ends
190190
// it. Measured that way on an unbuilt workspace with the reader gone — 27 of 30
191191
// cold-cache runs, main thread in `write(2)` on fd 2 at `sock_alloc_send_pskb`,
192-
// still alive at a 90 s ceiling. `stderr-nonblocking.mjs` carries the whole
192+
// still alive at a 90 s ceiling. `src/utils/stderr-nonblocking.ts` carries the whole
193193
// derivation, including who clears the flag (a child spawned with inherited
194194
// stdio — libuv clears `O_NONBLOCK` on the SHARED open file description) and
195195
// why the re-assert has to sit on the write path rather than run once here.

‎packages/cli/bin/run.js‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
//
77
// It used to be `await execute({ type: 'esm', dir: import.meta.url })`. What is
88
// inlined below IS `execute()` from @oclif/core 4.13.3, verbatim apart from the
9-
// one added line, because `execute` swallows the error into `handle()` and
9+
// added lines, because `execute` swallows the error into `handle()` and
1010
// there is no hook between the two. `handle()` writes the parse error and then
1111
// a full usage dump; #10111 needs one unmistakable line to reach stderr FIRST,
1212
// so a backgrounded runner that skims its log reads "the command never ran"
@@ -37,6 +37,39 @@ async function announceInvocationFailure(error) {
3737
}
3838
}
3939

40+
// ⚠️ BEFORE `run()`, and that order is the whole point rather than tidiness.
41+
//
42+
// Node puts fd 2 on the NON-blocking path when it opens the pipe, and libuv
43+
// clears that flag again in the pre-exec of any child spawned with inherited
44+
// stdio — on the SHARED open file description, so the spawner loses it too.
45+
// This binary is the one that spawns: `os dev` starts `os serve --dev` with
46+
// inherited stdio, and that child starts the esbuild service with inherited
47+
// stderr. Measured on the built binary with its output piped to a reader that
48+
// stopped draining: fd 2 was left blocking from 5.2 s onward and the main
49+
// thread then sat in `write(2)` (`wchan=sock_alloc_send_pskb`) 3.1 s later, 4
50+
// of 4 runs — alive, idle, ignoring SIGINT, empty log, released only when the
51+
// consumer resumed. `src/utils/stderr-nonblocking.ts` carries the whole
52+
// derivation, including why the re-assert has to sit on the write path rather
53+
// than run once here: the clearing that persisted came from a GRANDCHILD this
54+
// process does not spawn and cannot see.
55+
//
56+
// Everything the CLI writes to stderr is written after this point — oclif's own
57+
// output starts inside `Config.load()`, i.e. inside `run()` — so a guard
58+
// installed here has not missed a write.
59+
//
60+
// Lazily imported for the reason `announceInvocationFailure` states above: a
61+
// STATIC `../dist/` import would turn an unbuilt tree's "command not found"
62+
// into a module-resolution error and break the classification every gate that
63+
// shells out to this CLI depends on. ⛔ The `catch` therefore degrades to the
64+
// behaviour this file had before the guard existed; it must never become a
65+
// report, because the only stream it could report on is the one being repaired.
66+
try {
67+
const { keepStderrNonBlocking } = await import('../dist/utils/stderr-nonblocking.js');
68+
keepStderrNonBlocking();
69+
} catch {
70+
// Unbuilt or half-built tree — nothing to install and nothing to say.
71+
}
72+
4073
await run(process.argv.slice(2), import.meta.url)
4174
.then(async (result) => {
4275
flush();

‎packages/cli/bin/stderr-nonblocking.mjs‎

Lines changed: 0 additions & 109 deletions
This file was deleted.
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Keep this process's stderr writes off the BLOCKING path for the whole run.
5+
*
6+
* Installed by BOTH CLI entry points — `bin/run.js` (the published `bin`
7+
* target) and `bin/run-dev.js` (the source shim) — immediately before `run()`.
8+
*
9+
* ## Why this module lives in `src/` and not beside the shims in `bin/`
10+
*
11+
* It was written in `bin/`, next to its only caller at the time. That put it
12+
* outside everything npm ships: `files` names `dist`, `README.md` and
13+
* `CHANGELOG.md`, and npm packs a `bin` TARGET regardless of `files` — which is
14+
* why `bin/run.js` reaches a published install and a sibling module beside it
15+
* does not. `npm pack --dry-run` listed `bin/run.js` as the only packed file
16+
* under `bin/`, so NO published install carried the guard at all while the
17+
* defect it prevents was reproducing on the shipped binary. Compiling from
18+
* `src/` puts it under the existing whitelist instead of widening it.
19+
*
20+
* ## The premise, and the measurement that made it false
21+
*
22+
* Node makes the premise true when it opens the pipe: `uv_pipe_open()` sets
23+
* `O_NONBLOCK` on fd 2 the moment `process.stderr` is first touched, so a write
24+
* to a reader that has stopped is BUFFERED rather than parking the thread
25+
* inside `write(2)`. What is easy to miss is that libuv CLEARS it again in the
26+
* pre-exec of every child spawned with inherited stdio
27+
* (`uv__process_child_init` does exactly that for fds 0-2, deliberately,
28+
* because a child expects blocking stdio) — and since inheriting is `dup2`, the
29+
* child shares the parent's OPEN FILE DESCRIPTION. The flag lives on the
30+
* description, not on the fd number, so clearing it for the child clears it for
31+
* the SPAWNER TOO.
32+
*
33+
* Measured on the PUBLISHED binary, `node bin/run.js dev --verbose` with its
34+
* output piped to a reader that stopped draining, sampling `/proc/PID/fdinfo/2`
35+
* from outside the process every 50 ms:
36+
*
37+
* ```
38+
* 112 ms fd2 O_NONBLOCK=true process.stderr materialised
39+
* 2829 ms fd2 O_NONBLOCK=false `os dev` spawns `os serve --dev`, stdio inherit
40+
* 2930 ms fd2 O_NONBLOCK=true the serve child materialised ITS OWN stdio
41+
* 5244 ms fd2 O_NONBLOCK=false the esbuild service, a GRANDCHILD, inherits stderr
42+
* ```
43+
*
44+
* and from there the main thread of the `os serve --dev` child never left
45+
* `write(2)`, 4 of 4 runs, 3.1 s after the reader stopped:
46+
*
47+
* ```
48+
* syscall=1(write) fd=2 O_NONBLOCK=false state=S wchan=sock_alloc_send_pskb
49+
* tid=… (node) syscall=1(write) ← the MAIN thread
50+
* ```
51+
*
52+
* Parked 28.9 s; SIGINT was IGNORED while parked (3 of 3, alive and still in
53+
* `write` five seconds later); released only when the reader resumed. With the
54+
* loop parked in the kernel there is no late timer to catch up and no signal
55+
* handler to run: the process is alive, idle, unresponsive, with an empty log.
56+
*
57+
* ## Why the re-assert is on the WRITE path and not done once at startup
58+
*
59+
* Because the clearing that PERSISTS is not made by this process. On the
60+
* published path it came from esbuild — spawned by the `os serve --dev` child,
61+
* two levels below the `os dev` parent, with only stderr inherited — 5.2 s into
62+
* the run, and no change to any of this CLI's own spawn sites would have
63+
* prevented it. The same holds under `tsx`, where the service is spawned when a
64+
* module has to be transformed (measured 27 of 30 on a cold transform cache
65+
* against 1 of 90 on a warm one). A one-shot at startup is undone by the next
66+
* such spawn — including from a module-hooks worker thread, which shares the
67+
* same descriptions — and it fails SILENTLY, back into the hang it was meant to
68+
* prevent. Re-asserting immediately before each write costs one `fcntl` and
69+
* cannot be outrun by a later spawn, whoever makes it.
70+
*
71+
* ⚠️ The reverse also happens and must not be mistaken for a fix: any Node
72+
* child that materialises its own stdio re-SETS the flag on the shared
73+
* description ~100 ms later. That is why fd 1 tends to escape and fd 2 does
74+
* not, and why "it was true the last time I looked" is worth nothing here.
75+
*
76+
* ## ⛔ This is not the prohibited call, it is its inverse
77+
*
78+
* `bin/run-dev.js` and `src/utils/format.ts` both refuse
79+
* `_handle.setBlocking(TRUE)`, and that refusal stands: forcing blocking writes
80+
* process-wide is what stalls `os serve` / `os dev` on its own logs. This
81+
* function forces the other direction — it is the thing that KEEPS those two
82+
* docblocks true when something else has quietly flipped the flag.
83+
*
84+
* ⛔ It does not touch a TTY. A terminal is written synchronously on POSIX by
85+
* design, has no unread-reader failure mode (the reader is a human's terminal),
86+
* and prompt-adjacent output would change behaviour for no benefit.
87+
*/
88+
89+
/** Marks the stream so a second install cannot stack wrappers. */
90+
const INSTALLED = Symbol.for('objectstack.stderr-nonblocking');
91+
92+
/**
93+
* The libuv handle behind a stdio stream. Not in `@types/node` — it is an
94+
* internal — so the shape this module actually uses is named here rather than
95+
* reached for through `any`.
96+
*/
97+
interface BlockingCapableHandle {
98+
setBlocking(blocking: boolean): void;
99+
}
100+
101+
/** `stream.write`, seen as the plain callable this module re-invokes. */
102+
type StreamWrite = (...args: unknown[]) => boolean;
103+
104+
/**
105+
* @param stream The stream to guard; defaults to `process.stderr`.
106+
* Parameterised for the pin, which drives the guard against a manufactured
107+
* blocking pipe rather than waiting for a cold cache.
108+
* @returns `true` when this process's writes are now guarded, `false` when
109+
* there was nothing to guard (a TTY, a file, a stream with no libuv handle).
110+
* The boolean is returned rather than logged: a reporter that announces
111+
* itself on the very stream it is repairing is the one thing this file must
112+
* not do.
113+
*/
114+
export function keepStderrNonBlocking(stream: NodeJS.WriteStream = process.stderr): boolean {
115+
if (!stream || stream.isTTY === true) return false;
116+
const handle = (stream as unknown as { _handle?: BlockingCapableHandle | null })._handle;
117+
if (!handle || typeof handle.setBlocking !== 'function') return false;
118+
119+
const marks = stream as unknown as Record<symbol, boolean | undefined>;
120+
if (marks[INSTALLED]) return true;
121+
122+
const target = stream as unknown as { write: StreamWrite };
123+
const write = target.write;
124+
if (typeof write !== 'function') return false;
125+
126+
marks[INSTALLED] = true;
127+
target.write = function guardedWrite(this: unknown, ...args: unknown[]): boolean {
128+
// One `fcntl`, immediately ahead of the syscall that would otherwise park
129+
// this thread. Wrapped because a stream that lost its handle mid-run must
130+
// still take the write — a repair that throws is worse than the defect.
131+
try {
132+
handle.setBlocking(false);
133+
} catch {
134+
// Nothing to say and nowhere safe to say it.
135+
}
136+
return write.apply(this, args);
137+
};
138+
return true;
139+
}

0 commit comments

Comments
 (0)