fix(tui): suppress unmanaged macOS stderr writes corrupting the viewport
On macOS, libmalloc writes runtime diagnostics (e.g. "MallocStackLogging: can't turn off malloc stack logging because it was not enabled") directly to fd 2 of the running TUI process at arbitrary times, painting into the viewport. The existing env-strip only protects child processes. Add a fd-level stderr guard in pi-utils (suppressTerminalStderr / restoreTerminalStderr) that dup2-redirects fd 2 to the omp log file while the TUI owns the terminal, and restores it at every ownership handoff (external editor, Ctrl+Z suspend, shutdown, crash restore). Postmortem fatal handlers restore fd 2 before printing so crash reports stay visible. Mirrors openai/codex#24459.
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed macOS runtime diagnostics (e.g. `MallocStackLogging: can't turn off malloc stack logging because it was not enabled`) written directly to fd 2 by libmalloc painting into the TUI viewport. While the TUI owns the terminal, stderr is now redirected to the omp log file and restored at every ownership handoff (external editor, Ctrl+Z suspend, shutdown, crash restore); fatal crash reports still reach the real terminal. Mirrors [openai/codex#24459](https://github.com/openai/codex/pull/24459).
|
||||
|
||||
## [16.4.2] - 2026-07-10
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed unmanaged macOS stderr writes (libmalloc/framework diagnostics) corrupting the viewport: `ProcessTerminal` now suppresses fd 2 via the pi-utils stderr guard while it owns the terminal and restores it in `stop()` and the emergency-restore path.
|
||||
|
||||
## [16.4.1] - 2026-07-10
|
||||
|
||||
### Added
|
||||
|
||||
@@ -1,6 +1,14 @@
|
||||
import { dlopen, FFIType, ptr } from "bun:ffi";
|
||||
import * as fs from "node:fs";
|
||||
import { $env, isBunTestRuntime, isTerminalHeadless, logger, postmortem } from "@oh-my-pi/pi-utils";
|
||||
import {
|
||||
$env,
|
||||
isBunTestRuntime,
|
||||
isTerminalHeadless,
|
||||
logger,
|
||||
postmortem,
|
||||
restoreTerminalStderr,
|
||||
suppressTerminalStderr,
|
||||
} from "@oh-my-pi/pi-utils";
|
||||
import { setKittyProtocolActive } from "./keys";
|
||||
import { StdinBuffer } from "./stdin-buffer";
|
||||
import {
|
||||
@@ -275,6 +283,9 @@ function createConsoleCodepageGuard(): (() => void) | null {
|
||||
*/
|
||||
export function emergencyTerminalRestore(): void {
|
||||
try {
|
||||
// Crash paths must surface subsequent stderr (fatal reports) on the
|
||||
// real terminal; no-op when the stderr guard is inactive.
|
||||
restoreTerminalStderr();
|
||||
const terminal = activeTerminal;
|
||||
if (terminal) {
|
||||
terminal.stop();
|
||||
@@ -551,6 +562,11 @@ export class ProcessTerminal implements Terminal {
|
||||
activeTerminal = this;
|
||||
terminalEverStarted = true;
|
||||
|
||||
// Keep unmanaged fd-2 writes (macOS libmalloc/framework diagnostics) off
|
||||
// the viewport while we own the terminal; released in stop(). See
|
||||
// stderr-guard in pi-utils (mirrors openai/codex#24459).
|
||||
suppressTerminalStderr();
|
||||
|
||||
// Save previous state and enable raw mode
|
||||
this.#wasRaw = process.stdin.isRaw || false;
|
||||
if (process.stdin.setRawMode) {
|
||||
@@ -1269,6 +1285,11 @@ export class ProcessTerminal implements Terminal {
|
||||
activeTerminal = null;
|
||||
}
|
||||
|
||||
// Release terminal ownership of fd 2 first so external programs,
|
||||
// suspend, and shutdown see the real stderr even if a later teardown
|
||||
// step throws.
|
||||
restoreTerminalStderr();
|
||||
|
||||
if (this.#clearProgressTimer()) {
|
||||
this.#safeWrite(TERMINAL_PROGRESS_CLEAR_SEQUENCE);
|
||||
}
|
||||
|
||||
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Added a terminal stderr guard (`suppressTerminalStderr`/`restoreTerminalStderr`): dup2-redirects fd 2 to the omp log file while a TUI owns the terminal so macOS runtime diagnostics cannot paint into the viewport. The postmortem fatal handlers restore fd 2 before printing so crash reports stay visible.
|
||||
|
||||
## [16.4.2] - 2026-07-10
|
||||
|
||||
### Added
|
||||
|
||||
@@ -26,6 +26,7 @@ export { AbortError, ChildProcess, Exception, NonZeroExitError } from "./ptree";
|
||||
export * from "./runtime-install";
|
||||
export * from "./sanitize-text";
|
||||
export * from "./snowflake";
|
||||
export * from "./stderr-guard";
|
||||
export * from "./stream";
|
||||
export * from "./tab-spacing";
|
||||
export * from "./temp";
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
import inspector from "node:inspector";
|
||||
import { isMainThread } from "node:worker_threads";
|
||||
import { logger } from ".";
|
||||
import { restoreTerminalStderr } from "./stderr-guard";
|
||||
|
||||
// Cleanup reasons, in order of priority/meaning.
|
||||
export enum Reason {
|
||||
@@ -166,6 +167,11 @@ if (isMainThread) {
|
||||
logger.warn("Ignoring expected cleanup exception", { err });
|
||||
return;
|
||||
}
|
||||
// fd 2 may be redirected to the log while a TUI owns the terminal
|
||||
// (stderr-guard); re-point it at the real terminal so the fatal
|
||||
// report is visible. Terminal modes are restored moments later by
|
||||
// the terminal-restore cleanup callback inside runCleanup().
|
||||
restoreTerminalStderr();
|
||||
process.stderr.write(formatFatalError("Uncaught Exception", err));
|
||||
logger.error("Uncaught exception", { err });
|
||||
await runCleanup(Reason.UNCAUGHT_EXCEPTION);
|
||||
@@ -199,6 +205,8 @@ if (isMainThread) {
|
||||
});
|
||||
}
|
||||
}
|
||||
// See uncaughtException above: surface the report on the real stderr.
|
||||
restoreTerminalStderr();
|
||||
process.stderr.write(formatFatalError("Unhandled Rejection", err));
|
||||
logger.error("Unhandled rejection", { err });
|
||||
await runCleanup(Reason.UNHANDLED_REJECTION);
|
||||
|
||||
@@ -0,0 +1,149 @@
|
||||
/**
|
||||
* Terminal stderr guard: keeps unmanaged fd-2 writes off the terminal while a
|
||||
* TUI owns the viewport.
|
||||
*
|
||||
* On macOS, runtime diagnostics are written by the platform directly to file
|
||||
* descriptor 2 at arbitrary times — e.g. libmalloc's "MallocStackLogging:
|
||||
* can't turn off malloc stack logging because it was not enabled" when the OS
|
||||
* broadcasts a memory-diagnostic event to long-lived processes. Those bytes
|
||||
* bypass the renderer and paint straight into the viewport. Stripping the
|
||||
* MallocStackLogging* env vars (cli.ts) only protects child processes; it
|
||||
* cannot stop libmalloc inside THIS process from logging.
|
||||
*
|
||||
* Fix (mirrors openai/codex#24459): while the TUI owns the terminal, dup fd 2
|
||||
* aside and dup2 a redirect target over it; restore the saved fd whenever
|
||||
* terminal ownership is released (external editor, Ctrl+Z suspend, shutdown,
|
||||
* crash restore). Unlike codex we redirect to the omp log file — not
|
||||
* /dev/null — so the diagnostics stay greppable and Bun native-crash reports
|
||||
* (which abort before any JS cleanup can restore fd 2) are preserved.
|
||||
*
|
||||
* Only dup/dup2 go through bun:ffi. fcntl is deliberately avoided: it is
|
||||
* variadic, and the arm64-darwin ABI passes variadic arguments on the stack,
|
||||
* so a fixed-arity FFI signature would read garbage for the third argument.
|
||||
*/
|
||||
import { dlopen, FFIType } from "bun:ffi";
|
||||
import * as fs from "node:fs";
|
||||
import { getLogPath } from "./dirs";
|
||||
|
||||
const STDOUT_FILENO = 1;
|
||||
const STDERR_FILENO = 2;
|
||||
|
||||
interface LibcFdOps {
|
||||
dup(fd: number): number;
|
||||
dup2(oldFd: number, newFd: number): number;
|
||||
}
|
||||
|
||||
let libcFdOpsCache: LibcFdOps | null | undefined;
|
||||
|
||||
function libcFdOps(): LibcFdOps | null {
|
||||
if (libcFdOpsCache !== undefined) return libcFdOpsCache;
|
||||
libcFdOpsCache = null;
|
||||
if (process.platform === "win32") return null;
|
||||
// Darwin: dyld resolves libSystem from the shared cache. Linux: glibc
|
||||
// first, then the generic soname for musl-style layouts.
|
||||
const candidates =
|
||||
process.platform === "darwin" ? ["libSystem.B.dylib", "/usr/lib/libSystem.B.dylib"] : ["libc.so.6", "libc.so"];
|
||||
for (const candidate of candidates) {
|
||||
try {
|
||||
const libc = dlopen(candidate, {
|
||||
dup: { args: [FFIType.i32], returns: FFIType.i32 },
|
||||
dup2: { args: [FFIType.i32, FFIType.i32], returns: FFIType.i32 },
|
||||
});
|
||||
libcFdOpsCache = libc.symbols;
|
||||
return libcFdOpsCache;
|
||||
} catch {
|
||||
// Try the next candidate; the guard stays inert if none load.
|
||||
}
|
||||
}
|
||||
return libcFdOpsCache;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when fd 2 writes would land on the same terminal the TUI paints to:
|
||||
* both stdout and stderr are ttys backed by the same device file. A stderr
|
||||
* the user already redirected (`2>file`, `2>/dev/null`, a different tty) must
|
||||
* keep flowing untouched.
|
||||
*/
|
||||
function stderrSharesStdoutTerminal(): boolean {
|
||||
if (!process.stdout.isTTY || !process.stderr.isTTY) return false;
|
||||
try {
|
||||
const stdoutStat = fs.fstatSync(STDOUT_FILENO);
|
||||
const stderrStat = fs.fstatSync(STDERR_FILENO);
|
||||
return stdoutStat.dev === stderrStat.dev && stdoutStat.ino === stderrStat.ino;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Saved dup of the real stderr while suppression is active, else null. */
|
||||
let savedStderrFd: number | null = null;
|
||||
|
||||
export interface SuppressTerminalStderrOptions {
|
||||
/** Redirect target path; defaults to today's omp log file, then /dev/null. */
|
||||
redirectPath?: string;
|
||||
/** Bypass the macOS + same-terminal gate. Tests only. */
|
||||
force?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Redirect fd 2 away from the terminal while the TUI owns the viewport.
|
||||
* Returns true when suppression is (already) active. No-op — returning
|
||||
* false — off macOS, when stderr does not target the stdout terminal, or
|
||||
* when the libc fd ops are unavailable.
|
||||
*/
|
||||
export function suppressTerminalStderr(options?: SuppressTerminalStderrOptions): boolean {
|
||||
if (savedStderrFd !== null) return true;
|
||||
if (!options?.force && (process.platform !== "darwin" || !stderrSharesStdoutTerminal())) {
|
||||
return false;
|
||||
}
|
||||
const libc = libcFdOps();
|
||||
if (!libc) return false;
|
||||
|
||||
let redirectFd: number;
|
||||
try {
|
||||
redirectFd = fs.openSync(options?.redirectPath ?? getLogPath(), "a");
|
||||
} catch {
|
||||
try {
|
||||
redirectFd = fs.openSync("/dev/null", "w");
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
const saved = libc.dup(STDERR_FILENO);
|
||||
if (saved === -1) {
|
||||
fs.closeSync(redirectFd);
|
||||
return false;
|
||||
}
|
||||
if (libc.dup2(redirectFd, STDERR_FILENO) === -1) {
|
||||
fs.closeSync(redirectFd);
|
||||
fs.closeSync(saved);
|
||||
return false;
|
||||
}
|
||||
fs.closeSync(redirectFd);
|
||||
savedStderrFd = saved;
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-point fd 2 at the saved terminal stderr. Safe to call unconditionally:
|
||||
* no-op when suppression is not active. Called at every terminal-ownership
|
||||
* release and by the postmortem fatal handlers before they print, so crash
|
||||
* reports reach the real terminal.
|
||||
*/
|
||||
export function restoreTerminalStderr(): void {
|
||||
if (savedStderrFd === null) return;
|
||||
const saved = savedStderrFd;
|
||||
savedStderrFd = null;
|
||||
libcFdOps()?.dup2(saved, STDERR_FILENO);
|
||||
try {
|
||||
fs.closeSync(saved);
|
||||
} catch {
|
||||
// The dup'ed fd is process-owned; a close failure leaves nothing to recover.
|
||||
}
|
||||
}
|
||||
|
||||
/** Whether fd 2 is currently redirected away from the terminal. */
|
||||
export function isTerminalStderrSuppressed(): boolean {
|
||||
return savedStderrFd !== null;
|
||||
}
|
||||
@@ -0,0 +1,97 @@
|
||||
import { afterEach, describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
|
||||
/**
|
||||
* Contract (issue: macOS libmalloc diagnostics painting into the TUI
|
||||
* viewport; mirrors openai/codex#24459): while suppression is active, fd-2
|
||||
* writes land in the redirect target instead of the previous stderr; restore
|
||||
* rejoins the saved stderr; without `force`, a stderr that is not the stdout
|
||||
* terminal (here: a pipe) is left untouched.
|
||||
*
|
||||
* Runs in a subprocess so the test suite's own fd 2 is never mutated.
|
||||
*/
|
||||
|
||||
const GUARD_MODULE = path.resolve(import.meta.dir, "../src/stderr-guard.ts");
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
for (const dir of tempDirs.splice(0)) {
|
||||
fs.rmSync(dir, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
interface ProbeReport {
|
||||
gateResult: boolean;
|
||||
forced: boolean;
|
||||
secondSuppress: boolean;
|
||||
suppressedWhileActive: boolean;
|
||||
suppressedAfterRestore: boolean;
|
||||
}
|
||||
|
||||
describe("stderr guard", () => {
|
||||
it("suppresses fd-2 writes only while active and refuses non-terminal stderr without force", async () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "stderr-guard-"));
|
||||
tempDirs.push(dir);
|
||||
const redirectPath = path.join(dir, "redirect.log");
|
||||
const probePath = path.join(dir, "probe.ts");
|
||||
fs.writeFileSync(
|
||||
probePath,
|
||||
[
|
||||
`import { isTerminalStderrSuppressed, restoreTerminalStderr, suppressTerminalStderr } from ${JSON.stringify(GUARD_MODULE)};`,
|
||||
`import * as fs from "node:fs";`,
|
||||
`const redirectPath = process.argv[2];`,
|
||||
`fs.writeSync(2, "before\\n");`,
|
||||
`// stderr is a pipe here, so the same-terminal gate must refuse.`,
|
||||
`const gateResult = suppressTerminalStderr();`,
|
||||
`const forced = suppressTerminalStderr({ force: true, redirectPath });`,
|
||||
`const suppressedWhileActive = isTerminalStderrSuppressed();`,
|
||||
`if (forced) fs.writeSync(2, "hidden\\n");`,
|
||||
`// Idempotent while active: must not stack a second saved fd.`,
|
||||
`const secondSuppress = suppressTerminalStderr({ force: true, redirectPath });`,
|
||||
`restoreTerminalStderr();`,
|
||||
`fs.writeSync(2, "after\\n");`,
|
||||
`// Restore without active suppression is a no-op.`,
|
||||
`restoreTerminalStderr();`,
|
||||
`fs.writeSync(2, "still-visible\\n");`,
|
||||
`process.stdout.write(JSON.stringify({`,
|
||||
` gateResult,`,
|
||||
` forced,`,
|
||||
` secondSuppress,`,
|
||||
` suppressedWhileActive,`,
|
||||
` suppressedAfterRestore: isTerminalStderrSuppressed(),`,
|
||||
`}));`,
|
||||
].join("\n"),
|
||||
);
|
||||
|
||||
const proc = Bun.spawn([process.execPath, probePath, redirectPath], {
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
});
|
||||
const [stdout, stderr, exitCode] = await Promise.all([
|
||||
new Response(proc.stdout as ReadableStream<Uint8Array>).text(),
|
||||
new Response(proc.stderr as ReadableStream<Uint8Array>).text(),
|
||||
proc.exited,
|
||||
]);
|
||||
|
||||
expect(exitCode).toBe(0);
|
||||
const report = JSON.parse(stdout) as ProbeReport;
|
||||
// Piped stderr is not the stdout terminal → the non-forced gate refuses.
|
||||
expect(report.gateResult).toBe(false);
|
||||
expect(report.suppressedAfterRestore).toBe(false);
|
||||
|
||||
if (report.forced) {
|
||||
expect(report.suppressedWhileActive).toBe(true);
|
||||
expect(report.secondSuppress).toBe(true);
|
||||
expect(stderr).toBe("before\nafter\nstill-visible\n");
|
||||
expect(fs.readFileSync(redirectPath, "utf8")).toBe("hidden\n");
|
||||
} else {
|
||||
// libc fd ops unavailable on this platform: the guard must stay
|
||||
// inert and every write must reach the original stderr.
|
||||
expect(stderr).toBe("before\nafter\nstill-visible\n");
|
||||
expect(fs.existsSync(redirectPath)).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user