From 722ac3aa5a6a683092db0798fe2e777d37a98dbb Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 17 Dec 2025 18:34:58 -0800 Subject: [PATCH] fix: check correct variable in subprocess stdin cleanup (#25562) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Fix typo in `onProcessExit` where `existing_stdin_value.isCell()` was checked instead of `existing_value.isCell()` - Since `existing_stdin_value` is always `.zero` at that point, the condition was always false, making the inner block dead code ## The Bug In `src/bun.js/api/bun/subprocess.zig:593`: ```zig var existing_stdin_value = jsc.JSValue.zero; // Line 590 - always .zero if (this_jsvalue != .zero) { if (jsc.Codegen.JSSubprocess.stdinGetCached(this_jsvalue)) |existing_value| { if (existing_stdin_value.isCell()) { // BUG! Should be existing_value // This block was DEAD CODE - never executed ``` Compare with the correct pattern used elsewhere: ```zig // shell/subproc.zig:251-252 (CORRECT) if (jsc.Codegen.JSSubprocess.stdinGetCached(subprocess.this_jsvalue)) |existing_value| { jsc.WebCore.FileSink.JSSink.setDestroyCallback(existing_value, 0); // Uses existing_value } ``` ## Impact The dead code prevented: - Recovery of stdin from cached JS value when `weak_file_sink_stdin_ptr` is null - Proper cleanup via `onAttachedProcessExit` on the FileSink - `setDestroyCallback` cleanup in `onProcessExit` Note: The user-visible impact was mitigated by redundant cleanup paths in `Writable.zig` that also call `setDestroyCallback`. ## Test plan - Code inspection confirms this is a straightforward typo fix - Existing subprocess tests continue to pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude --- src/bun.js/api/bun/subprocess.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/api/bun/subprocess.zig b/src/bun.js/api/bun/subprocess.zig index f1fc61e2b2..b9b2e03918 100644 --- a/src/bun.js/api/bun/subprocess.zig +++ b/src/bun.js/api/bun/subprocess.zig @@ -590,7 +590,7 @@ pub fn onProcessExit(this: *Subprocess, process: *Process, status: bun.spawn.Sta var existing_stdin_value = jsc.JSValue.zero; if (this_jsvalue != .zero) { if (jsc.Codegen.JSSubprocess.stdinGetCached(this_jsvalue)) |existing_value| { - if (existing_stdin_value.isCell()) { + if (existing_value.isCell()) { if (stdin == null) { // TODO: review this cast stdin = @ptrCast(@alignCast(jsc.WebCore.FileSink.JSSink.fromJS(existing_value)));