WalkthroughReorders and consolidates VirtualMachine shutdown: removes immediate per-thread destructor calls in globalExit, adds explicit teardown of the forever timer and GC controller in deinit, and makes transpiler and auto_killer deinitialization conditional on running on the main thread. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
You can enable these sources in your CodeRabbit configuration.
src/bun.js/VirtualMachine.zig (1 hunks)📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/bun.js/VirtualMachine.zig📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/bun.js/VirtualMachine.zig📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy
Files:
src/bun.js/VirtualMachine.zigLearnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In finalize() for objects holding JS references, release them using .deref() before destroy
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/**/js_*.zig : Always implement proper cleanup in deinit() and finalize() for JS-exposed types
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Applied to files:
src/bun.js/VirtualMachine.zigLearnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In finalize() for objects holding JS references, release them using .deref() before destroy
Applied to files:
src/bun.js/VirtualMachine.zigLearnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/**/js_*.zig : Always implement proper cleanup in deinit() and finalize() for JS-exposed types
Applied to files:
src/bun.js/VirtualMachine.zigThere was a problem hiding this comment.
Actionable comments posted: 1
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
You can enable these sources in your CodeRabbit configuration.
src/bun.js/VirtualMachine.zig (2 hunks)📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/bun.js/VirtualMachine.zig📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/bun.js/VirtualMachine.zig📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy
Files:
src/bun.js/VirtualMachine.zigLearnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Applied to files:
src/bun.js/VirtualMachine.zigLearnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/**/js_*.zig : Always implement proper cleanup in deinit() and finalize() for JS-exposed types
Applied to files:
src/bun.js/VirtualMachine.zigLearnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In finalize() for objects holding JS references, release them using .deref() before destroy
Applied to files:
src/bun.js/VirtualMachine.zigLearnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to src/bun.js/bindings/ZigGlobalObject.cpp : Initialize the LazyClassStructure in GlobalObject::finishCreation and visit it in GlobalObject::visitChildrenImpl
Applied to files:
src/bun.js/VirtualMachine.zig| if (this.is_main_thread) { | ||
| this.transpiler.deinit(); | ||
| this.auto_killer.deinit(); | ||
| } |
There was a problem hiding this comment.
is_main_thread guard never fires
is_main_thread defaults to false on the struct and none of the construction paths (init, initWithModuleGraph, initWorker, etc.) ever flip it to true. With this check in place we now skip both transpiler.deinit() and auto_killer.deinit() for every VM, regressing cleanup and leaking a lot more than the 72 B you’re chasing. Please gate on this.isMainThread() (or ensure the flag is set) so we still tear these resources down.
- if (this.is_main_thread) {
+ if (this.isMainThread()) {
this.transpiler.deinit();
this.auto_killer.deinit();
}
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (this.is_main_thread) { | |
| this.transpiler.deinit(); | |
| this.auto_killer.deinit(); | |
| } | |
| if (this.isMainThread()) { | |
| this.transpiler.deinit(); | |
| this.auto_killer.deinit(); | |
| } |
In src/bun.js/VirtualMachine.zig around lines 1930 to 1933, the current guard
uses the struct field this.is_main_thread which defaults to false and is never
set, so the cleanup for transpiler and auto_killer never runs; change the guard
to call this.isMainThread() (or alternatively ensure all VM constructors set
this.is_main_thread = true for main-thread VMs) so that transpiler.deinit() and
auto_killer.deinit() are invoked when appropriate and resources are torn down.
There was a problem hiding this comment.
In src/bun.js/VirtualMachine.zig around lines 1930 to 1933, the current guard uses the struct field this.is_main_thread which defaults to false and is never set, so the cleanup for transpiler and auto_killer never runs; change the guard to call this.isMainThread() (or alternatively ensure all VM constructors set this.is_main_thread = true for main-thread VMs) so that transpiler.deinit() and auto_killer.deinit() are invoked when appropriate and resources are torn down
There was a problem hiding this comment.
In src/bun.js/VirtualMachine.zig around lines 1930 to 1933, the current guard uses the struct field this.is_main_thread which defaults to false and is never set, so the cleanup for transpiler and auto_killer never runs; change the guard to call this.isMainThread() (or alternatively ensure all VM constructors set this.is_main_thread = true for main-thread VMs) so that transpiler.deinit() and auto_killer.deinit() are invoked when appropriate and resources are torn down
There was a problem hiding this comment.
@zJesterz, you're absolutely correct! I've verified that is_main_thread is never assigned to true anywhere in the codebase. While the constructors receive opts.is_main_thread and use it to set VMHolder.main_thread_vm, they never actually set vm.is_main_thread = true. This means the cleanup guard will always be false, causing transpiler.deinit() and auto_killer.deinit() to never run—defeating the purpose of this cleanup code.
The fix is straightforward: use the existing isMainThread() method which correctly checks return this.worker == null;.
Learnt from: alii
Repo: oven-sh/bun PR: 26402
File: src/bundler/linker_context/generateChunksInParallel.zig:307-355
Timestamp: 2026-01-23T22:52:42.507Z
Learning: In Bun's bundler linker context (src/bundler/linker_context/*.zig), module_info operations for ESM bytecode caching use `catch continue` / `catch {}` to silently handle allocation failures because module_info is supplementary metadata, not critical to producing correct JavaScript output. Critical allocations use `bun.handleOom()` or `try`, but module_info operations (mi.str(), mi.addVar(), mi.requestModule(), mi.addImportInfoSingle(), etc.) intentionally use graceful degradation to avoid crashing builds on OOM for non-critical metadata. This pattern is used consistently across postProcessJSChunk.zig, generateChunksInParallel.zig, and related files.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T23:38:11.972Z
Learning: Applies to src/**/*.zig : In Zig code, be careful with allocators and use `defer` for cleanup
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:286-289
Timestamp: 2025-10-19T03:01:29.084Z
Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path.
<!-- [/add_learning]
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use JSC::WriteBarrier for heap-allocated references in V8 objects and implement visitChildren() for custom heap objects to support garbage collection
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T23:38:11.972Z
Learning: Applies to src/**/*.zig : When making platform-specific changes, run `bun run zig:check-all` to compile the Zig code on all platforms
Learnt from: robobun
Repo: oven-sh/bun PR: 0
File: :0-0
Timestamp: 2026-01-27T15:03:53.182Z
Learning: In Bun's MessagePort implementation (src/bun.js/bindings/webcore/MessagePort.cpp), the native `tryTakeMessage` function returns `jsUndefined()` in two indistinguishable cases: (1) when no message is available (line 306), and (2) when a message deserializes to undefined (line 310). This creates a fundamental limitation where the JavaScript wrapper in worker_threads.ts cannot distinguish between "no message" and "message with undefined value". Only distinguishable falsy values (0, false, '', null) can be correctly handled; fixing the undefined case would require native API changes.
<!-- </add_learning>
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/allocators/allocation_scope.zig:284-314
Timestamp: 2025-09-02T18:25:27.976Z
Learning: In bun's custom Zig implementation, the `#` prefix for private fields is valid syntax and should not be flagged as invalid. The syntax `#fieldname` creates private fields that cannot be accessed from outside the defining struct, and usage like `self.#fieldname` is correct within the same struct. This applies to fields like `#parent`, `#state`, `#allocator`, `#trace`, etc. throughout the codebase.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T23:38:11.972Z
Learning: Applies to src/bun.js/bindings/*.cpp : In JavaScript class implementations in C++, cache structures in ZigGlobalObject
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/memory.zig:37-43
Timestamp: 2025-09-02T18:29:58.304Z
Learning: In bun's `src/memory.zig`, the `initDefault` function intentionally calls `T.init()` without parameter validation to cause compile-time errors when `init` requires arguments or returns an error union. This design forces developers to add explicit `initDefault` methods for complex types rather than silently falling back to `{}`. The `{}` fallback is only intended for types that have no `init` function at all.
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey.zig:0-0
Timestamp: 2025-09-05T18:45:29.200Z
Learning: In JSValkeyClient.cloneWithoutConnecting() in src/valkey/js_valkey.zig, the address/username/password fields must be repointed to the duplicated connection_strings buffer to avoid use-after-free when the original client is destroyed. The original client properly frees connection_strings in ValkeyClient.deinit().
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/memory.zig:60-76
Timestamp: 2025-09-02T17:41:07.869Z
Learning: In bun's memory utilities, when handling const pointers in deinit operations, prefer compile-time errors over silent skipping to avoid hard-to-find memory leaks. Users expect explicit failures rather than silent omissions in memory management.
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:15.175Z
Learning: In Bun's Zig codebase, std.posix.sigaction returns void (not an error union) and should be called without error handling. The Zig standard library treats sigaction failures as programmer errors (unreachable) since they only occur with invalid signals like SIGKILL/SIGSTOP. This pattern is used consistently across crash_handler.zig, main.zig, filter_run.zig, and process.zig.
💡 For code changes across multiple files, use @coderabbitai plan in an issue comment
There was a problem hiding this comment.
🏁 Script executed:
#!/bin/bash
# Search for assignments to is_main_thread field
echo "=== Searching for 'is_main_thread =' assignments ==="
rg -n --type=zig 'is_main_thread\s*=' src/
echo -e "\n=== Searching for '.is_main_thread = true' patterns ==="
rg -n --type=zig '\.is_main_thread\s*=\s*true' src/
echo -e "\n=== Checking how isMainThread() method is implemented ==="
rg -n --type=zig -A3 'pub fn isMainThread\(' src/bun.js/VirtualMachine.zig
Length of output: 1334
@zJesterz Good catch! You're right that using the isMainThread() method is more robust here.
While I can see that is_main_thread is being set in various calling code paths (like src/bun.js.zig, src/cli/test_command.zig, etc.) after VM construction, relying on this field is fragile because:
false and must be explicitly assigned by calling codeisMainThread() checks the runtime state (this.worker == null) rather than depending on all code paths correctly setting the fieldThe isMainThread() method is the safer choice here since it directly checks whether this is a worker VM, which is the actual condition we care about.
Recommendation: Change the guard to use the method:
if (this.isMainThread()) {
this.transpiler.deinit();
this.auto_killer.deinit();
}This ensures transpiler and auto_killer are properly cleaned up for main thread VMs regardless of whether calling code remembered to set the field.
Learnt from: alii
Repo: oven-sh/bun PR: 26402
File: src/bundler/linker_context/generateChunksInParallel.zig:307-355
Timestamp: 2026-01-23T22:52:42.507Z
Learning: In Bun's bundler linker context (src/bundler/linker_context/*.zig), module_info operations for ESM bytecode caching use `catch continue` / `catch {}` to silently handle allocation failures because module_info is supplementary metadata, not critical to producing correct JavaScript output. Critical allocations use `bun.handleOom()` or `try`, but module_info operations (mi.str(), mi.addVar(), mi.requestModule(), mi.addImportInfoSingle(), etc.) intentionally use graceful degradation to avoid crashing builds on OOM for non-critical metadata. This pattern is used consistently across postProcessJSChunk.zig, generateChunksInParallel.zig, and related files.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T23:38:11.972Z
Learning: Applies to src/**/*.zig : In Zig code, be careful with allocators and use `defer` for cleanup
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:286-289
Timestamp: 2025-10-19T03:01:29.084Z
Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path.
<!-- [/add_learning]
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/V8*.cpp : Use JSC::WriteBarrier for heap-allocated references in V8 objects and implement visitChildren() for custom heap objects to support garbage collection
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/memory.zig:60-76
Timestamp: 2025-09-02T17:41:07.869Z
Learning: In bun's memory utilities, when handling const pointers in deinit operations, prefer compile-time errors over silent skipping to avoid hard-to-find memory leaks. Users expect explicit failures rather than silent omissions in memory management.
Learnt from: robobun
Repo: oven-sh/bun PR: 0
File: :0-0
Timestamp: 2026-01-27T15:03:53.182Z
Learning: In Bun's MessagePort implementation (src/bun.js/bindings/webcore/MessagePort.cpp), the native `tryTakeMessage` function returns `jsUndefined()` in two indistinguishable cases: (1) when no message is available (line 306), and (2) when a message deserializes to undefined (line 310). This creates a fundamental limitation where the JavaScript wrapper in worker_threads.ts cannot distinguish between "no message" and "message with undefined value". Only distinguishable falsy values (0, false, '', null) can be correctly handled; fixing the undefined case would require native API changes.
<!-- </add_learning>
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/bindings/JSValue.zig:545-586
Timestamp: 2025-11-03T20:40:59.655Z
Learning: In Bun's Zig codebase, JSErrors (returned as `bun.JSError!T`) must always be properly handled. Using `catch continue` or `catch { break; }` to silently suppress JSErrors is a bug. Errors should either be explicitly handled or propagated with `try`. This applies to snapshot serializer error handling where Jest's behavior is to throw when serializers throw.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T23:38:11.972Z
Learning: Applies to src/bun.js/bindings/*.cpp : In JavaScript class implementations in C++, cache structures in ZigGlobalObject
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey.zig:0-0
Timestamp: 2025-09-05T18:45:29.200Z
Learning: In JSValkeyClient.cloneWithoutConnecting() in src/valkey/js_valkey.zig, the address/username/password fields must be repointed to the duplicated connection_strings buffer to avoid use-after-free when the original client is destroyed. The original client properly frees connection_strings in ValkeyClient.deinit().
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22227
File: src/memory.zig:37-43
Timestamp: 2025-09-02T18:29:58.304Z
Learning: In bun's `src/memory.zig`, the `initDefault` function intentionally calls `T.init()` without parameter validation to cause compile-time errors when `init` requires arguments or returns an error union. This design forces developers to add explicit `initDefault` methods for complex types rather than silently falling back to `{}`. The `{}` fallback is only intended for types that have no `init` function at all.
Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:15.175Z
Learning: In Bun's Zig codebase, std.posix.sigaction returns void (not an error union) and should be called without error handling. The Zig standard library treats sigaction failures as programmer errors (unreachable) since they only occur with invalid signals like SIGKILL/SIGSTOP. This pattern is used consistently across crash_handler.zig, main.zig, filter_run.zig, and process.zig.
|
BotNeT |
What does this PR do?
How did you verify your code works?