Skip to content

fix(shell): fix use-after-free in shell interpreter error path on Windows - #26839

Closed
robobun wants to merge 1 commit into
mainfrom
claude/fix-shell-interpreter-double-free
Closed

robobun wants to merge 1 commit into
mainfrom
claude/fix-shell-interpreter-double-free

Conversation

@robobun

@robobun robobun commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fix use-after-free / double-free crash in the shell interpreter when setupIOBeforeRun() fails in the JS-managed code path (runFromJS)
  • The error path was calling #deinitFromExec() which directly frees the interpreter via allocator.destroy(this), but since the interpreter is GC-managed in this code path, the GC finalizer would later access already-freed memory causing a segfault

Root cause

In runFromJS() (line 1156), when setupIOBeforeRun() returns an error, the code called #deinitFromExec() which:

  1. Frees root_io via deref()
  2. Frees root_shell via deinitImpl()
  3. Frees the interpreter itself via allocator.destroy(this)

However, when called from runFromJS, the interpreter is a GC-managed object. The GC finalizer (deinitFromFinalizer) would later attempt to access cleanup_state, call root_io.deref() and root_shell.deinitImpl() again, and call allocator.destroy(this) again — all on already-freed memory.

Fix

Replace #deinitFromExec() with #derefRootShellAndIOIfNeeded(true) which:

  1. Cleans up only runtime resources (IO, shell env)
  2. Sets cleanup_state = .runtime_cleaned so the GC finalizer knows not to double-free
  3. Does not call allocator.destroy(this) — lets the GC finalizer handle that

This is consistent with how the normal completion path (finish()) already handles cleanup.

Closes #26836

Test plan

  • Debug build compiles successfully
  • Existing shell tests pass (bunshell.test.ts: 340 pass, 4 pre-existing failures in unrelated "quiet" tests)
  • Shell error tests pass (throw.test.ts: 4 pass)
  • Regression test passes (22650-shell-crash.test.ts: 1 pass)
  • The crash is Windows-specific and timing-dependent (requires setupIOBeforeRun() to fail + GC to run), so it cannot be reliably reproduced in a unit test

🤖 Generated with Claude Code

…dows

When `runFromJS()` encounters an IO setup failure, it was calling
`#deinitFromExec()` which directly frees the interpreter via
`allocator.destroy(this)`. However, in the JS code path the interpreter
is a GC-managed object — the GC finalizer (`deinitFromFinalizer`) will
later attempt to access and free the already-freed memory, causing a
use-after-free / segfault.

Replace `#deinitFromExec()` with `#derefRootShellAndIOIfNeeded(true)`
which properly cleans up only runtime resources (IO, shell env) and
marks `cleanup_state = .runtime_cleaned`, allowing the GC finalizer
to safely handle the final destruction of the interpreter object.

Closes #26836

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the claude label Feb 9, 2026
@robobun

robobun commented Feb 9, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:00 PM PT - Feb 9th, 2026

❌ Your commit aeddfca1 has 3 failures in Build #36769 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 26839

That installs a local version of the PR into your bun-26839 executable, so you can run:

bun-26839 --bun

@coderabbitai

coderabbitai Bot commented Feb 9, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Modified error handling in runFromJS() to defer cleanup of runtime resources instead of immediately destroying the interpreter during IO setup failures, preventing use-after-free scenarios.

Changes

Cohort / File(s) Summary
Interpreter cleanup sequence
src/shell/interpreter.zig
Changed error path in runFromJS() during IO setup to call derefRootShellAndIOIfNeeded(true) instead of deinitFromExec(), deferring resource cleanup to garbage collection finalization rather than immediate interpreter destruction.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: preventing use-after-free in the shell interpreter error path on Windows.
Description check ✅ Passed The PR description provides clear context on the root cause, the fix, and test verification, matching the template's requirements.
Linked Issues check ✅ Passed The PR directly addresses issue #26836 by preventing double-free in the GC finalizer through proper cleanup state management.
Out of Scope Changes check ✅ Passed All changes are focused on the error path cleanup in runFromJS(), directly addressing the use-after-free issue without introducing unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


No actionable comments were generated in the recent review. 🎉


Comment @coderabbitai help to get the list of available commands and usage tips.

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

This is a duplicate PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segmentation fault at address 0x207353E0080

2 participants