Commit Graph

13344 Commits

Author SHA1 Message Date
Alistair Smith
ccc8a24081 tests 2025-08-20 13:17:38 -07:00
Alistair Smith
db5fcf16da move to post-resolution during install 2025-08-20 13:17:38 -07:00
Alistair Smith
f045dfecf4 tests 2025-08-20 13:17:38 -07:00
Alistair Smith
5093ad89e1 fix 2025-08-20 13:17:38 -07:00
Alistair Smith
e84be1da7c works 2025-08-20 13:17:37 -07:00
Alistair Smith
59f4158981 no integrity 2025-08-20 13:17:37 -07:00
Alistair Smith
d75845325b changes 2025-08-20 13:17:37 -07:00
Alistair Smith
c3d05ea3d9 check 2025-08-20 13:17:37 -07:00
Alistair Smith
dd30128099 import 2025-08-20 13:17:37 -07:00
Alistair Smith
5ed3f1b21e change 2025-08-20 13:17:37 -07:00
Alistair Smith
7ef2f047f9 revert 2025-08-20 13:17:37 -07:00
autofix-ci[bot]
98519215b5 [autofix.ci] apply automated fixes 2025-08-20 13:17:37 -07:00
Alistair Smith
28e3b88108 for now 2025-08-20 13:17:37 -07:00
Alistair Smith
c2ea7b3c10 test 2025-08-20 13:17:37 -07:00
Alistair Smith
505ed59d97 fix 2025-08-20 13:17:37 -07:00
autofix-ci[bot]
55336c2d93 [autofix.ci] apply automated fixes 2025-08-20 13:17:37 -07:00
Alistair Smith
5d18e47190 rough initial impl 2025-08-20 13:17:37 -07:00
Alistair Smith
fc30e6936a test 2025-08-20 13:17:37 -07:00
Alistair Smith
6b669ea431 test update 2025-08-20 13:17:37 -07:00
Alistair Smith
91d0930e68 does error 2025-08-20 13:17:37 -07:00
Alistair Smith
7e0458bc23 add not install 2025-08-20 13:17:37 -07:00
Alistair Smith
0cf158b486 msg 2025-08-20 13:17:37 -07:00
Alistair Smith
7628d105d4 bunfig related 2025-08-20 13:17:37 -07:00
Alistair Smith
233198999a this didn't work 2025-08-20 13:17:37 -07:00
Alistair Smith
72e1cad342 update test 2025-08-20 13:17:37 -07:00
Alistair Smith
2e7a55188b Some high level changes/loose test 2025-08-20 13:17:36 -07:00
Alistair Smith
f279855ad6 update comments 2025-08-20 13:17:36 -07:00
Alistair Smith
c534d67119 comment 2025-08-20 13:17:36 -07:00
Alistair Smith
1856124e28 sketch types for Bun.Install.Security 2025-08-20 13:17:34 -07:00
Michael H
d354714791 Plugins + cross-compilation + Bun.build API support for Bun.build({compile}) (#21915)
### What does this PR do?

in the name

### How did you verify your code works?

tests, but using ci to see if anything else broke

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
2025-08-20 01:25:49 -07:00
robobun
e7672b2d04 Add string fast path for postMessage and structuredClone (#21926)
## Summary

Implements a string fast path optimization for `postMessage` and
`structuredClone` operations that provides significant performance
improvements for string-only data transfer, along with various bug fixes
and infrastructure improvements.

## Key Performance Improvements

**postMessage with Workers:**
- **Small strings (11 chars):** ~5% faster (572ns vs 599ns)
- **Medium strings (14KB):** **~2.7x faster** (528ns vs 1.40μs) 
- **Large strings (3MB):** **~660x faster** (540ns vs 356μs)

**Compared to Node.js postMessage:**
- Similar performance for small strings
- Competitive for medium strings  
- **~455x faster** for large strings (540ns vs 245μs)

## Implementation Details

The optimization adds a **string fast path** that bypasses full
structured cloning serialization when:
- Input is a pure string (`value.isString()`)
- No transfer list or message ports are involved
- Not being stored persistently

### Core Changes

**String Thread-Safety Utilities (`BunString.cpp/h`):**
- `isCrossThreadShareable()` - Checks if string can be safely shared
across threads
- `toCrossThreadShareable()` - Converts strings to thread-safe form via
`isolatedCopy()`
- Handles edge cases: atoms, symbols, substring slices, external buffers

**Serialization Fast Path (`SerializedScriptValue.cpp`):**
- New `m_fastPathString` field stores string data directly
- Bypasses full object serialization machinery for pure strings
- Creates isolated copies for cross-thread safety

**Deserialization Fast Path:**
- Directly returns JSString from stored string data
- Avoids parsing serialized byte streams

**Updated Flags System (`JSValue.zig`, `Serialization.cpp`):**
- Replaces boolean `forTransfer` with structured `SerializedFlags`
- Supports `forCrossProcessTransfer` and `forStorage` distinctions

**Structured Clone Infrastructure:**
- Moved `structuredClone` implementation to dedicated
`StructuredClone.cpp`
- Added `jsFunctionStructuredCloneAdvanced` for testing with custom
flags
- Improved class serialization compatibility checks (`isForTransfer`,
`isForStorage`)

**IPC Improvements (`ipc.zig`):**
- Fixed race conditions in `SendQueue` by deferring cleanup to next tick
- Proper fd ownership handling with `bun.take()`
- Cached IPC serialize/parse functions for better performance

**BlockList Thread Safety Fixes (`BlockList.zig`):**
- Fixed potential deadlocks by moving mutex locking inside methods
- Added atomic `estimated_size` counter to avoid lock during GC
- Corrected pointer handling in comparison functions
- Improved GC safety in `rules()` method

## Benchmark Results

```
❯ bun-21926 bench/string-postmessage.mjs  # This branch
postMessage(11 chars string)  572.24 ns/iter
postMessage(14 KB string)     527.55 ns/iter  ← ~2.7x faster
postMessage(3 MB string)      539.70 ns/iter  ← ~660x faster

❯ bun-1.2.20 bench/string-postmessage.mjs  # Previous
postMessage(11 chars string)  598.76 ns/iter
postMessage(14 KB string)       1.40 µs/iter
postMessage(3 MB string)      356.38 µs/iter

❯ node bench/string-postmessage.mjs       # Node.js comparison  
postMessage(11 chars string)  569.63 ns/iter
postMessage(14 KB string)       1.46 µs/iter
postMessage(3 MB string)      245.46 µs/iter
```

**Key insight:** The fast path achieves **constant time performance**
regardless of string size (~540ns), while traditional serialization
scales linearly with data size.

## Test Coverage

**New Tests:**
- `test/js/web/structured-clone-fastpath.test.ts` - Fast path memory
usage validation
- `test/js/web/workers/structuredClone-classes.test.ts` - Comprehensive
class serialization tests
  - Tests ArrayBuffer transferability 
  - Tests BunFile cloning with storage/transfer restrictions
  - Tests net.BlockList cloning behavior
  - Validates different serialization contexts (default, worker, window)

**Enhanced Tests:**
- `test/js/web/workers/structured-clone.test.ts` - Multi-function
testing
- Tests `structuredClone`, `jscSerializeRoundtrip`, and cross-process
serialization
  - Validates consistency across different serialization paths
- `test/js/node/cluster.test.ts` - Better error handling and debugging

**Benchmarks:**
- `bench/string-postmessage.mjs` - Worker postMessage performance
comparison
- `bench/string-fastpath.mjs` - Fast path vs traditional serialization
comparison

## Bug Fixes

**BlockList Threading Issues:**
- Fixed potential deadlocks when multiple threads access BlockList
simultaneously
- Moved mutex locks inside methods rather than holding across entire
function calls
- Added atomic size tracking for GC compatibility
- Fixed comparison function pointer handling

**IPC Race Conditions:**
- Fixed race condition where `SendQueue._onAfterIPCClosed()` could be
called on wrong thread
- Deferred cleanup operations to next tick using task queue
- Improved file descriptor ownership with proper `bun.take()` usage

**Structured Clone Compatibility:**
- Enhanced class serialization with proper transfer/storage mode
checking
- Fixed edge cases where non-transferable objects were incorrectly
handled
- Added better error reporting for unsupported clone operations

## Technical Notes

- Thread safety ensured via `String.isolatedCopy()` for cross-VM
transfers
- Memory cost calculation updated to account for string references
- Maintains full compatibility with existing structured clone semantics
- Does not affect object serialization or transfer lists
- Proper cleanup and error handling throughout IPC pipeline

---------

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Meghan Denny <meghan@bun.sh>
2025-08-20 00:25:00 -07:00
robobun
7110dc10a4 Fix UTF-16 encoding crash with odd-length byte arrays (#21966)
## Summary
- Fixes a panic: "exact division produced remainder" that occurs when
reading files with odd number of bytes using utf16le/ucs2 encoding
- The crash happened in `encoding.zig:136` when
`std.mem.bytesAsSlice(u16, input)` was called on a byte slice with odd
length
- Fixed by properly checking for odd-length input and truncating to the
nearest even length

## Test plan
- Added regression tests in
`test/regression/issue/utf16-encoding-crash.test.ts`
- Tests verify that reading files with odd byte counts doesn't crash
- Tests verify correct truncation behavior matches Node.js expectations
- Verified edge cases (0, 1 byte inputs) return empty strings

## Root Cause
The original code checked `if (input.len / 2 == 0)` which only caught 0
and 1-byte inputs, but `std.mem.bytesAsSlice(u16, input)` panics on any
odd-length input (3, 5, 7, etc. bytes).

## Fix Details
- Changed condition to check `input.len % 2 != 0` for any odd length
- Truncate odd-length inputs to the nearest even length for valid UTF-16
processing
- Handle edge cases by returning empty string for 0 or 1-byte inputs

🤖 Generated with [Claude Code](https://claude.ai/code)

---------

Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
2025-08-20 00:02:14 -07:00
Alistair Smith
784271f85e SQLite in Bun.sql (#21640)
### What does this PR do?

Support sqlite in the Bun.sql API

Fixes #18951
Fixes #19701

### How did you verify your code works?

tests

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
2025-08-19 23:15:53 -07:00
Jarred Sumner
9b363e4ef6 Skip this test for now 2025-08-19 21:51:25 -07:00
Jarred Sumner
e6f6a487a1 Revert "Call JSC::VM::notifyNeedTermination on process.exit in a Web Worker" (#21994)
Reverts oven-sh/bun#21962

`vm.ensureTerminationException` allocates a JSString, which is not safe
to do from a thread that doesn't own the API lock.

```ts
Bun Canary v1.2.21-canary.1 (f706382a) Linux x64 (baseline)
Linux Kernel v6.12.38 | musl
CPU: sse42 popcnt avx avx2 avx512
Args: "/var/lib/buildkite-agent/builds/ip-172-31-38-185/bun/bun/release/bun-linux-x64-musl-baseline-profile/bun-profile" "/var/lib/buildkite-agent/builds/ip-172-31-38-185/bun/bun/test/js/node/worker_threads"...
Features: bunfig http_server jsc tsconfig(3) tsconfig_paths workers_spawned(40) workers_terminated(34)
Builtins: "bun:main" "node:worker_threads"
Elapsed: 362ms | User: 518ms | Sys: 63ms
RSS: 0.34GB | Peak: 100.36MB | Commit: 0.34GB | Faults: 0 | Machine: 8.17GB
 
panic(main thread): Segmentation fault at address 0x0
oh no: Bun has crashed. This indicates a bug in Bun, not your code.
 
To send a redacted crash report to Bun's team,
please file a GitHub issue using the link below:
 
 http://localhost:38809/1.2.21/Ba2f706382wNgkgUu11luEm6yX+lwy+Dgtt+oEurthoD8214mE___07+09DA2AA
 
 
 6 | describe("Worker destruction", () => {
 7 |   const method = ["Bun.connect", "Bun.listen", "fetch"];
 8 |   describe.each(method)("bun when %s is used in a Worker that is terminating", method => {
 9 |     // fetch: ASAN failure
10 |     test.skipIf(isBroken && method == "fetch")("exits cleanly", () => {
11 |       expect([join(import.meta.dir, "worker_thread_check.ts"), method]).toRun();
                                                                             ^
error:
 
Command /var/lib/buildkite-agent/builds/ip-172-31-38-185/bun/bun/test/js/node/worker_threads/worker_thread_check.ts Bun.connect failed:
Spawned 10 workers RSS 79 MB
Spawned 10 workers RSS 87 MB
Spawned 10 workers RSS 90 MB
 
      at <anonymous> (/var/lib/buildkite-agent/builds/ip-172-31-38-185/bun/bun/test/js/node/worker_threads/worker_destruction.test.ts:11:73)
✗ Worker destruction > bun when Bun.connect is used in a Worker that is terminating > exits cleanly [597.56ms]
✓ Worker destruction > bun when Bun.listen is used in a Worker that is terminating > exits cleanly [503.47ms]
» Worker destruction > bun when fetch is used in a Worker that is terminating > exits cleanly
 
 
 1 pass
 1 skip
 1 fail
 2 expect() calls
Ran 3 tests across 1 file. [1125.00ms]
======== Stack trace from GDB for bun-profile-28234.core: ========
Program terminated with signal SIGILL, Illegal instruction.
#0  crash_handler.crash () at crash_handler.zig:1523
[Current thread is 1 (LWP 28234)]
#0  crash_handler.crash () at crash_handler.zig:1523
#1  0x0000000002db77aa in crash_handler.crashHandler (reason=..., error_return_trace=0x0, begin_addr=...) at crash_handler.zig:471
#2  0x0000000002db2b55 in crash_handler.handleSegfaultPosix (sig=<optimized out>, info=<optimized out>) at crash_handler.zig:792
#3  0x0000000004716b58 in WTF::jscSignalHandler (sig=11, info=0x7ffe54051e90, ucontext=0x0) at vendor/WebKit/Source/WTF/wtf/threads/Signals.cpp:548
#4  <signal handler called>
#5  JSC::VM::currentThreadIsHoldingAPILock (this=0x148296c30000) at vendor/WebKit/Source/JavaScriptCore/runtime/VM.h:840
#6  JSC::sanitizeStackForVM (vm=...) at vendor/WebKit/Source/JavaScriptCore/runtime/VM.cpp:1369
#7  0x0000000003f4a060 in JSC::LocalAllocator::allocate(JSC::Heap&, unsigned long, JSC::GCDeferralContext*, JSC::AllocationFailureMode)::{lambda()#1}::operator()() const (this=<optimized out>) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/LocalAllocatorInlines.h:46
#8  JSC::FreeList::allocateWithCellSize<JSC::LocalAllocator::allocate(JSC::Heap&, unsigned long, JSC::GCDeferralContext*, JSC::AllocationFailureMode)::{lambda()#1}>(JSC::LocalAllocator::allocate(JSC::Heap&, unsigned long, JSC::GCDeferralContext*, JSC::AllocationFailureMode)::{lambda()#1} const&, unsigned long) (this=0x148296c38e48, cellSize=16, slowPath=...) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/FreeListInlines.h:46
#9  JSC::LocalAllocator::allocate (this=0x148296c38e30, heap=..., cellSize=16, deferralContext=0x0, failureMode=JSC::AllocationFailureMode::Assert) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/LocalAllocatorInlines.h:44
#10 JSC::GCClient::IsoSubspace::allocate (this=0x148296c38e30, vm=..., cellSize=16, deferralContext=0x0, failureMode=JSC::AllocationFailureMode::Assert) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/IsoSubspaceInlines.h:34
#11 JSC::tryAllocateCellHelper<JSC::JSString, (JSC::AllocationFailureMode)0> (vm=..., size=16, deferralContext=0x0) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/JSCellInlines.h:192
#12 JSC::allocateCell<JSC::JSString> (vm=..., size=16) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/JSCellInlines.h:212
#13 JSC::JSString::create (vm=..., value=...) at cache/webkit-a73e665a39b281c5/include/JavaScriptCore/JSString.h:204
#14 0x0000000004479ad1 in JSC::jsNontrivialString (vm=..., s=...) at vendor/WebKit/Source/JavaScriptCore/runtime/JSString.h:846
#15 JSC::VM::ensureTerminationException (this=0x148296c30000) at vendor/WebKit/Source/JavaScriptCore/runtime/VM.cpp:627
#16 JSGlobalObject__requestTermination (globalObject=<optimized out>) at ./build/release/./src/bun.js/bindings/ZigGlobalObject.cpp:3979
#17 0x0000000003405ab8 in bun.js.web_worker.notifyNeedTermination (this=0x542904f0d80) at /var/lib/buildkite-agent/builds/ip-172-31-16-28/bun/bun/src/bun.js/web_worker.zig:558
#18 0x0000000004362b6f in WebCore::Worker::terminate (this=0x984c900000000000) at ./src/bun.js/bindings/webcore/Worker.cpp:266
#19 WebCore::jsWorkerPrototypeFunction_terminateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSWorker*)::{lambda()#1}::operator()() const (this=<optimized out>) at ./build/release/./src/bun.js/bindings/webcore/JSWorker.cpp:549
#20 WebCore::toJS<WebCore::IDLUndefined, WebCore::jsWorkerPrototypeFunction_terminateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSWorker*)::{lambda()#1}>(JSC::JSGlobalObject&, JSC::ThrowScope&, WebCore::jsWorkerPrototypeFunction_terminateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSWorker*)::{lambda()#1}&&) (lexicalGlobalObject=..., throwScope=..., valueOrFunctor=...) at ./src/bun.js/bindings/webcore/JSDOMConvertBase.h:174
#21 WebCore::jsWorkerPrototypeFunction_terminateBody (lexicalGlobalObject=<optimized out>, callFrame=<optimized out>, castedThis=<optimized out>) at ./build/release/./src/bun.js/bindings/webcore/JSWorker.cpp:549
#22 WebCore::IDLOperation<WebCore::JSWorker>::call<&WebCore::jsWorkerPrototypeFunction_terminateBody, (WebCore::CastedThisErrorBehavior)0> (lexicalGlobalObject=..., operationName=..., callFrame=...) at ./src/bun.js/bindings/webcore/JSDOMOperation.h:63
#23 WebCore::jsWorkerPrototypeFunction_terminate (lexicalGlobalObject=<optimized out>, callFrame=0x7ffe540536b8) at ./build/release/./src/bun.js/bindings/webcore/JSWorker.cpp:554
#24 0x000014825580c038 in ?? ()
#25 0x00007ffe540537b0 in ?? ()
#26 0x0000148255a626cb in ?? ()
#27 0x0000000000000000 in ?? ()
1 crashes reported during this test
```
2025-08-19 20:49:48 -07:00
Jarred Sumner
2d86f46e07 Make exception checker clear for bun:sqlite tests (#21774)
### What does this PR do?

### How did you verify your code works?
2025-08-19 18:53:34 -07:00
robobun
f5ef9cda3c Fix panic in JavaScript lexer when parsing invalid template strings in JSX (#21967)
## Summary

- Fixes a crash where invalid slice bounds caused a panic with message:
"start index N is larger than end index M"
- The issue occurred in `js_lexer.zig:767` when calculating string
literal content slice bounds
- Adds proper bounds checking to prevent slice bounds violations
- Includes regression test to prevent future occurrences

## Root Cause

The crash happened when `suffix_len` was larger than `lexer.end`,
causing the calculation `lexer.end - suffix_len` to result in a value
smaller than the `base` position. This created invalid slice bounds like
`[114..113]`.

## Solution

Added bounds checking to ensure:
1. `end_pos` is calculated safely: `if (lexer.end >= suffix_len)
lexer.end - suffix_len else lexer.end`
2. `slice_end` is always >= `base`: `@max(base, end_pos)`

## Test Plan

- [x] Added regression test in
`test/regression/issue/jsx-template-string-crash.test.ts`
- [x] Test verifies no crashes occur with JSX template string patterns
- [x] Verified normal template string functionality still works
- [x] All tests pass

🤖 Generated with [Claude Code](https://claude.ai/code)

---------

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2025-08-19 18:47:04 -07:00
pfg
9ad5d3c6c3 Fix issue with Error.prepareStackTrace (#21829)
Fixes #21815

---------

Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2025-08-19 18:08:00 -07:00
SUZUKI Sosuke
decf84c416 Prevent namespace objects from inheriting Object.prototype (#21984)
### What does this PR do?

Fixes namespace import objects inheriting from `Object.prototype`,
preventing prototype pollution and ensuring ES specification compliance.

```js
import * as mod from './mod.mjs'

Object.prototype.foo = function() {
    console.log('hello');
}

mod.foo(); // This should throw, but succeeded before
```

original report: https://x.com/sapphi_red/status/1957843865722863876

### How did you verify your code works?

I added a test that verifies:

- `mod.maliciousFunction()` throws when
`Object.prototype.maliciousFunction` is added (prevents pollution)
- `__esModule` property still works
- Original exports remain accessible

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2025-08-19 16:40:48 -07:00
Meghan Denny
2d36588609 ci: set BUN_JSC_dumpSimulatedThrows in asan runs 2025-08-19 16:39:49 -07:00
Kai Tamkun
67e44e25e8 Call JSC::VM::notifyNeedTermination on process.exit in a Web Worker (#21962)
### What does this PR do?

Calling `process.exit` inside a Web Worker now immediately notifies the
VM that it needs to terminate. Previously, `napi_call_function` would
return success in a Web Worker even if the JS code called
`process.exit`. Now it'll return `napi_pending_exception`.

### How did you verify your code works?

Ran Node's NAPI tests (`node-api/test_worker_terminate/test.js` now
passes) in addition to our own NAPI tests.
2025-08-18 20:32:46 -07:00
Jarred Sumner
d1562a7670 Fix flickering in bun updated --interactive (#21964)
### What does this PR do?

Use
https://gist.github.com/christianparpart/d8a62cc1ab659194337d73e399004036
like we do in `bun run --filter`

### How did you verify your code works?

tried in next.js repo and in debug build it no longer flickers

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2025-08-18 20:02:08 -07:00
Jarred Sumner
287c7adf76 github action 2025-08-18 18:10:46 -07:00
Jarred Sumner
0a13653707 github action 2025-08-18 17:13:12 -07:00
Jarred Sumner
a2c252256c github action 2025-08-18 17:10:17 -07:00
Jarred Sumner
45c34beb65 github action 2025-08-18 17:07:10 -07:00
Jarred Sumner
a2da900e40 github action 2025-08-18 17:02:38 -07:00
Jarred Sumner
883453391f github action 2025-08-18 16:55:29 -07:00
robobun
0dc136149d fix(napi): prevent finalizer crash during process exit (#21951)
## Summary

Fixes a critical segmentation fault crash occurring during NAPI
finalizer cleanup when finalizers trigger GC operations. This crash was
reported with `node-sqlite3` and other NAPI modules during process exit.

## Root Cause

The crash was caused by **iterator invalidation** in
`napi_env__::cleanup()`:

1. Range-based for loop iterates over `m_finalizers`
(std::unordered_set)
2. `boundFinalizer.call(this)` executes finalizer callbacks
3. Finalizers can trigger JavaScript execution and GC operations  
4. GC can add/remove entries from `m_finalizers` during iteration
5. **Iterator invalidation** → undefined behavior → segfault

## Solution

Added `JSC::DeferGCForAWhile deferGC(m_vm)` scope during entire
finalizer cleanup to prevent any GC operations from occurring during
iteration.

### Why This Approach?

- **Addresses root cause**: Prevents GC entirely during critical section
- **Simple & safe**: One-line RAII fix using existing JSC patterns  
- **Minimal impact**: Only affects process cleanup (not runtime
performance)
- **Proven pattern**: Already used elsewhere in Bun's codebase
- **Better than alternatives**: Cleaner than Node.js manual iterator
approach

## Testing

Added comprehensive test (`test_finalizer_iterator_invalidation.c`) that
reproduces the crash by:
- Creating finalizers that trigger GC operations
- Forcing JavaScript execution during finalization
- Allocating objects that can trigger more GC
- Calling process exit to trigger finalizer cleanup

**Before fix**: Segmentation fault  
**After fix**: Clean exit 

## Files Changed

- `src/bun.js/bindings/napi.h`: Core fix + include
- `test/napi/napi-app/test_finalizer_iterator_invalidation.c`: Test
addon
- `test/napi/napi-app/binding.gyp`: Build config for test addon
- `test/regression/issue/napi-finalizer-crash.test.ts`: Integration test

## Test Plan

- [x] Reproduces original crash without fix
- [x] Passes cleanly with fix applied  
- [x] Existing NAPI tests continue to pass
- [x] Manual testing with node-sqlite3 scenarios

🤖 Generated with [Claude Code](https://claude.ai/code)

---------

Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Kai Tamkun <kai@tamkun.io>
2025-08-18 16:48:48 -07:00
robobun
8526b2512e fix: napi_is_exception_pending crash during cleanup (#21961)
## Summary

Fixes a crash in `napi_is_exception_pending` that occurs during
environment cleanup when finalizers call this function.

The crash manifested as:
```
panic: Aborted
- napi.h:192: napi_is_exception_pending  
- napi.h:516: wrap_cleanup
- napi.h:273: napi_env__::cleanup
```

## Root Cause

Bun's implementation was using `DECLARE_THROW_SCOPE` during cleanup when
JavaScript execution is not safe, and didn't follow Node.js's approach
of avoiding `NAPI_PREAMBLE` for this function.

## Changes Made

1. **Remove `NAPI_PREAMBLE_NO_THROW_SCOPE`** - Node.js explicitly states
this function "must execute when there is a pending exception"
2. **Use `DECLARE_CATCH_SCOPE`** instead of `DECLARE_THROW_SCOPE` for
safety during cleanup
3. **Add safety check** `!env->isFinishingFinalizers()` before accessing
VM
4. **Add `napi_clear_last_error` function** to match Node.js
implementation
5. **Use `napi_clear_last_error`** instead of `napi_set_last_error` for
consistent behavior

## Test Plan

Created comprehensive test that:
-  **Reproduces the original crash scenario** (finalizers calling
`napi_is_exception_pending`)
-  **Verifies it no longer crashes in Bun** 
-  **Confirms behavior matches Node.js exactly**

### Test Results

**Before fix:** Would crash with `panic: Aborted` during cleanup

**After fix:** 
```
Testing napi_is_exception_pending behavior...

1. Testing basic napi_is_exception_pending:
   Status: 0 (should be 0 for napi_ok)
   Result: false (should be false - no exception pending)

2. Testing with pending exception:
   Exception was thrown as expected: Test exception

3. Testing finalizer scenario (the crash case):
   Creating object with finalizer that calls napi_is_exception_pending...
   Objects created. Forcing garbage collection...
   Garbage collection completed.
napi_is_exception_pending in finalizer: status=0, result=false
[...5 finalizers ran successfully...]

SUCCESS: napi_is_exception_pending works correctly in all scenarios!
```

**Node.js comparison:** Identical output and behavior confirmed.

## Impact

- **Fixes crashes** in native addons that call
`napi_is_exception_pending` in finalizers
- **Improves Node.js compatibility** by aligning implementation approach
- **No breaking changes** - only fixes crash scenario, normal usage
unchanged

The fix aligns Bun's NAPI implementation with Node.js's proven approach
for safe exception checking during environment cleanup.

🤖 Generated with [Claude Code](https://claude.ai/code)

---------

Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2025-08-18 16:47:45 -07:00