Add ExternalShared and RawRefCount#23013
Merged
Merged
Conversation
Add `bun.ptr.ExternalShared`, a shared pointer whose reference count is
managed externally; e.g., by extern functions. This can be used to work
with `RefCounted` C++ objects in Zig. For example:
```cpp
// C++:
struct MyType : RefCounted<MyType> { ... };
extern "C" void MyType__ref(MyType* self) { self->ref(); }
extern "C" void MyType__ref(MyType* self) { self->deref(); }
```
```zig
// Zig:
const MyType = opaque {
extern fn MyType__ref(self: *MyType) void;
extern fn MyType__deref(self: *MyType) void;
pub const Ref = bun.ptr.ExternalShared(MyType);
// This enables `ExternalShared` to work.
pub const external_shared_descriptor = struct {
pub const ref = MyType__ref;
pub const deref = MyType__deref;
};
};
// Now `MyType.Ref` behaves just like `Ref<MyType>` in C++:
var some_ref: MyType.Ref = someFunctionReturningMyTypeRef();
const ptr: *MyType = some_ref.get(); // gets the inner pointer
var some_other_ref = some_ref.clone(); // increments the ref count
some_ref.deinit(); // decrements the ref count
// decrements the ref count again; if no other refs exist, the object
// is destroyed
some_other_ref.deinit();
```
This commit also adds `RawRefCount`, a simple wrapper around an integer
reference count that can be used to implement the interface required by
`ExternalShared`. Generally, for reference-counted Zig types,
`bun.ptr.Shared` is preferred, but occasionally it is useful to have an
“intrusive” reference-counted type where the ref count is stored in the
type itself. For this purpose, `ExternalShared` + `RawRefCount` is more
flexible and less error-prone than the deprecated `bun.ptr.RefCounted`
type.
Collaborator
|
Updated 2:30 PM PT - Sep 26th, 2025
❌ @taylordotfish, your commit d965c2e has 2 failures in
🧪 To try this PR locally: bunx bun-pr 23013That installs a local version of the PR into your bun-23013 --bun |
Contributor
WalkthroughAdded a new ExternalShared smart pointer type with a nullable Optional variant, a RawRefCount generic thread-aware reference-count helper, re-exported both from Changes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)src/**/*.zig📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Files:
**/*.zig📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/ptr.zig(2 hunks)src/ptr/external_shared.zig(1 hunks)src/ptr/raw_ref_count.zig(1 hunks)src/string/WTFStringImpl.zig(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.zig
📄 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/string/WTFStringImpl.zigsrc/ptr.zigsrc/ptr/raw_ref_count.zigsrc/ptr/external_shared.zig
**/*.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/string/WTFStringImpl.zigsrc/ptr.zigsrc/ptr/raw_ref_count.zigsrc/ptr/external_shared.zig
🧠 Learnings (4)
📓 Common learnings
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 : Handle reference counting correctly with ref()/deref() in JS-facing Zig code
Learnt 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 **/*.zig : Declare the extern C symbol in Zig and export a Zig-friendly alias for use
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt 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 **/*.zig : Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Applied to files:
src/string/WTFStringImpl.zigsrc/ptr.zig
📚 Learning: 2025-08-30T00:11:57.076Z
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 : Handle reference counting correctly with ref()/deref() in JS-facing Zig code
Applied to files:
src/ptr.zigsrc/ptr/raw_ref_count.zig
📚 Learning: 2025-09-02T17:14:01.470Z
Learnt from: taylordotfish
PR: oven-sh/bun#22227
File: src/ptr/shared.zig:470-470
Timestamp: 2025-09-02T17:14:01.470Z
Learning: In Zig, regular arithmetic operations (like +=) have built-in overflow detection in debug/safe builds and will panic automatically. Atomic operations (like fetchAdd) do not have this automatic safety checking, so explicit overflow checks must be added manually. This is why NonAtomicCount doesn't need explicit overflow checks while AtomicCount does.
Applied to files:
src/ptr/raw_ref_count.zig
🔇 Additional comments (5)
src/ptr/external_shared.zig (1)
15-109: Rename#implfields to valid Zig identifiersZig identifiers cannot start with
#, so both the main struct and the optional wrapper fail to compile as written. Please rename these fields and update their usages accordingly.- #impl: *T, + ptr: *T, @@ - return .{ .#impl = incremented_raw }; + return .{ .ptr = incremented_raw }; @@ - T.external_shared_descriptor.deref(self.#impl); + T.external_shared_descriptor.deref(self.ptr); @@ - return self.#impl; + return self.ptr; @@ - T.external_shared_descriptor.ref(self.#impl); + T.external_shared_descriptor.ref(self.ptr); @@ - return .{ .#impl = raw }; + return .{ .ptr = raw }; @@ - defer self.* = undefined; - return self.#impl; + defer self.* = undefined; + return self.ptr; @@ - pub const Optional = struct { - #impl: ?*T = null, + pub const Optional = struct { + ptr: ?*T = null, @@ - return .{ .#impl = incremented_raw }; + return .{ .ptr = incremented_raw }; @@ - if (self.#impl) |impl| { + if (self.ptr) |impl| { @@ - return self.#impl; + return self.ptr; @@ - const result: NonOptional = .{ .#impl = self.#impl orelse return null }; - self.#impl = null; + const result: NonOptional = .{ .ptr = self.ptr orelse return null }; + self.ptr = null; @@ - if (self.#impl) |impl| { + if (self.ptr) |impl| { @@ - return .{ .#impl = raw }; + return .{ .ptr = raw }; @@ - defer self.* = undefined; - return self.#impl; + defer self.* = undefined; + return self.ptr; @@ - return .{ .#impl = self.#impl }; + return .{ .ptr = self.ptr };⛔ Skipped due to learnings
Learnt from: taylordotfish PR: oven-sh/bun#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: taylordotfish PR: oven-sh/bun#22227 File: src/safety/alloc.zig:93-95 Timestamp: 2025-09-02T17:14:46.924Z Learning: In bun's Zig codebase, they use a custom extension of Zig that supports private field syntax with the `#` prefix (e.g., `#allocator`, `#trace`). This is not standard Zig syntax but is valid in their custom implementation. Fields prefixed with `#` are private fields that cannot be accessed from outside the defining struct.Learnt from: cirospaciari PR: oven-sh/bun#22842 File: src/bun.js/webcore/ResumableSink.zig:274-276 Timestamp: 2025-09-25T18:14:27.722Z Learning: In Zig code, private fields are declared and accessed using the `#` prefix. When a field is declared as `#field_name`, it must be accessed as `this.#field_name`, not `this.field_name`. The `#` prefix is part of the private field access syntax and should not be removed.Learnt 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 **/*.zig : Declare the extern C symbol in Zig and export a Zig-friendly alias for useLearnt from: taylordotfish PR: oven-sh/bun#22227 File: src/collections/multi_array_list.zig:24-24 Timestamp: 2025-09-02T18:27:23.279Z Learning: The `#allocator` syntax in bun's custom Zig implementation is valid and does not require quoting with @"#allocator". Private fields using the `#` prefix work correctly throughout the codebase without special quoting syntax.src/ptr/raw_ref_count.zig (1)
19-53: Use valid identifier names instead of#thread_lockIdentifiers starting with
#are illegal in Zig, so this field (and all references to it) prevent the module from compiling. Rename the field to a standard identifier and update the call sites.- #thread_lock: if (thread_safety == .single_threaded) bun.safety.ThreadLock else void, + thread_lock: if (thread_safety == .single_threaded) bun.safety.ThreadLock else void, @@ - .#thread_lock = switch (comptime thread_safety) { + .thread_lock = switch (comptime thread_safety) { @@ - self.#thread_lock.lockOrAssert(); + self.thread_lock.lockOrAssert(); @@ - self.#thread_lock.lockOrAssert(); + self.thread_lock.lockOrAssert();⛔ Skipped due to learnings
Learnt from: taylordotfish PR: oven-sh/bun#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: cirospaciari PR: oven-sh/bun#22842 File: src/bun.js/webcore/ResumableSink.zig:274-276 Timestamp: 2025-09-25T18:14:27.722Z Learning: In Zig code, private fields are declared and accessed using the `#` prefix. When a field is declared as `#field_name`, it must be accessed as `this.#field_name`, not `this.field_name`. The `#` prefix is part of the private field access syntax and should not be removed.Learnt from: taylordotfish PR: oven-sh/bun#22227 File: src/safety/alloc.zig:93-95 Timestamp: 2025-09-02T17:14:46.924Z Learning: In bun's Zig codebase, they use a custom extension of Zig that supports private field syntax with the `#` prefix (e.g., `#allocator`, `#trace`). This is not standard Zig syntax but is valid in their custom implementation. Fields prefixed with `#` are private fields that cannot be accessed from outside the defining struct.Learnt from: taylordotfish PR: oven-sh/bun#22227 File: src/collections/multi_array_list.zig:24-24 Timestamp: 2025-09-02T18:27:23.279Z Learning: The `#allocator` syntax in bun's custom Zig implementation is valid and does not require quoting with @"#allocator". Private fields using the `#` prefix work correctly throughout the codebase without special quoting syntax.src/string/WTFStringImpl.zig (2)
221-224: Descriptor exposes the right hooksRe-exporting the existing
ref/derefensuresExternalSharedwill reuse the proven WTF ref-counting paths, so we stay in lockstep with WebKit semantics.
227-229: Alias linesWTFStringup with the new smart pointerThis keeps the public surface matching the old
WTF::Refbehavior while routing through the new intrusive wrapper—nice and tidy.src/ptr.zig (1)
16-28: Clean re-export of the new pointer utilitiesPulling
ExternalSharedandRawRefCountinto the central ptr module makes them discoverable alongside the rest of the smart-pointer toolbox.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/ptr/raw_ref_count.zig(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.zig
📄 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/ptr/raw_ref_count.zig
**/*.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/ptr/raw_ref_count.zig
🧠 Learnings (3)
📓 Common learnings
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 : Handle reference counting correctly with ref()/deref() in JS-facing Zig code
Learnt 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 **/*.zig : Declare the extern C symbol in Zig and export a Zig-friendly alias for use
📚 Learning: 2025-08-30T00:11:57.076Z
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 : Handle reference counting correctly with ref()/deref() in JS-facing Zig code
Applied to files:
src/ptr/raw_ref_count.zig
📚 Learning: 2025-09-02T17:14:01.470Z
Learnt from: taylordotfish
PR: oven-sh/bun#22227
File: src/ptr/shared.zig:470-470
Timestamp: 2025-09-02T17:14:01.470Z
Learning: In Zig, regular arithmetic operations (like +=) have built-in overflow detection in debug/safe builds and will panic automatically. Atomic operations (like fetchAdd) do not have this automatic safety checking, so explicit overflow checks must be added manually. This is why NonAtomicCount doesn't need explicit overflow checks while AtomicCount does.
Applied to files:
src/ptr/raw_ref_count.zig
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Format
taylordotfish
added a commit
that referenced
this pull request
Sep 26, 2025
Reduce the size of `bun.webcore.Blob` from 120 bytes to 96. Also make it ref-counted: in-progress work on improving the bindings generator depends on this, as it means C++ can pass a pointer to the `Blob` to Zig without risking it being destroyed if the GC collects the associated `JSBlob`. Note that this PR depends on #23013.
taylordotfish
added a commit
that referenced
this pull request
Sep 27, 2025
Reduce the size of `bun.webcore.Blob` from 120 bytes to 96. Also make it ref-counted: in-progress work on improving the bindings generator depends on this, as it means C++ can pass a pointer to the `Blob` to Zig without risking it being destroyed if the GC collects the associated `JSBlob`. Note that this PR depends on #23013. (For internal tracking: fixes STAB-1289, STAB-1290)
This was referenced Sep 27, 2025
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
bun.ptr.ExternalShared, a shared pointer whose reference count is managed externally; e.g., by extern functions. This can be used to work withRefCountedC++ objects in Zig. For example:This commit also adds
RawRefCount, a simple wrapper around an integer reference count that can be used to implement the interface required byExternalShared. Generally, for reference-counted Zig types,bun.ptr.Sharedis preferred, but occasionally it is useful to have an “intrusive” reference-counted type where the ref count is stored in the type itself. For this purpose,ExternalShared+RawRefCountis more flexible and less error-prone than the deprecatedbun.ptr.RefCountedtype.(For internal tracking: fixes STAB-1287, STAB-1288)