wip: start using Arc(Source) in Blob

This commit is contained in:
Don Isaac
2025-03-31 16:00:33 -07:00
parent 4ebde315ca
commit 8a1c911e7b
5 changed files with 98 additions and 45 deletions

View File

@@ -29,6 +29,7 @@ const JSPromise = JSC.JSPromise;
const JSValue = JSC.JSValue;
const JSGlobalObject = JSC.JSGlobalObject;
const NullableAllocator = bun.NullableAllocator;
const Arc = bun.ptr.Arc;
const VirtualMachine = JSC.VirtualMachine;
const Task = JSC.Task;
@@ -83,7 +84,7 @@ pub const Blob = struct {
/// When set, the blob will be freed on finalization callbacks
/// If the blob is contained in Response or Request, this must be null
allocator: ?std.mem.Allocator = null,
store: ?*Store = null,
store: ?Arc(Store) = null,
content_type: string = "",
content_type_allocated: bool = false,
content_type_was_set: bool = false,
@@ -124,7 +125,10 @@ pub const Blob = struct {
}
pub fn hasContentTypeFromUser(this: *const Blob) bool {
return this.content_type_was_set or (this.store != null and (this.store.?.data == .file or this.store.?.data == .s3));
return this.content_type_was_set or if (this.store) |store| switch (store.get().data) {
.s3, .file => true,
else => false,
} else false;
}
pub fn contentTypeOrMimeType(this: *const Blob) ?[]const u8 {
@@ -132,7 +136,7 @@ pub const Blob = struct {
return this.content_type;
}
if (this.store) |store| {
switch (store.data) {
switch (store.get().data) {
.file => |file| {
return file.mime_type.value;
},
@@ -148,7 +152,7 @@ pub const Blob = struct {
pub fn isBunFile(this: *const Blob) bool {
const store = this.store orelse return false;
return store.data == .file;
return store.get().data == .file;
}
pub fn doReadFromS3(this: *Blob, comptime Function: anytype, global: *JSGlobalObject) JSValue {
@@ -5183,7 +5187,7 @@ pub const Blob = struct {
return tryCreate(bytes_, allocator_, globalThis, was_string) catch bun.outOfMemory();
}
pub fn initWithStore(store: *Blob.Store, globalThis: *JSGlobalObject) Blob {
pub fn initWithStore(store: Arc(Blob.Store), globalThis: *JSGlobalObject) Blob {
return Blob{
.size = store.size(),
.store = store,
@@ -5213,7 +5217,7 @@ pub const Blob = struct {
}
pub fn detach(this: *Blob) void {
if (this.store != null) this.store.?.deref();
if (this.store) |*s| s.deinit();
this.store = null;
}
@@ -5225,7 +5229,8 @@ pub const Blob = struct {
}
pub fn dupeWithContentType(this: *const Blob, include_content_type: bool) Blob {
if (this.store != null) this.store.?.ref();
// SAFETY: creating a copy of `this`, so the new reference belongs to the new Blob.
if (this.store) |*s| s.dangerouslyIncrementRef();
var duped = this.*;
if (duped.content_type_allocated and duped.allocator != null and !include_content_type) {

View File

@@ -15,6 +15,7 @@ const JSPromise = JSC.JSPromise;
const JSGlobalObject = JSC.JSGlobalObject;
const ZigString = JSC.ZigString;
const libuv = bun.windows.libuv;
const Arc = bun.ptr.Arc;
const log = bun.Output.scoped(.ReadFile, true);
@@ -64,7 +65,7 @@ pub const ReadFileTask = JSC.WorkTask(ReadFile);
pub const ReadFile = struct {
file_store: FileStore,
byte_store: ByteStore = ByteStore{ .allocator = bun.default_allocator },
store: ?*Store = null,
store: ?Arc(Store) = null,
offset: SizeType = 0,
max_length: SizeType = Blob.max_size,
total_size: SizeType = Blob.max_size,
@@ -100,7 +101,7 @@ pub const ReadFile = struct {
pub fn createWithCtx(
_: std.mem.Allocator,
store: *Store,
store: Arc(Store),
onReadFileContext: *anyopaque,
onCompleteCallback: ReadFileOnReadFileCallback,
off: SizeType,
@@ -110,20 +111,20 @@ pub const ReadFile = struct {
@compileError("Do not call ReadFile.createWithCtx on Windows, see ReadFileUV");
const read_file = bun.new(ReadFile, ReadFile{
.file_store = store.data.file,
// TODO: do not store this separately from .store
.file_store = store.get().data.file,
.offset = off,
.max_length = max_len,
.store = store,
.onCompleteCtx = onReadFileContext,
.onCompleteCallback = onCompleteCallback,
});
store.ref();
return read_file;
}
pub fn create(
allocator: std.mem.Allocator,
store: *Store,
store: Arc(Store),
off: SizeType,
max_len: SizeType,
comptime Context: type,
@@ -258,14 +259,14 @@ pub const ReadFile = struct {
const cb = this.onCompleteCallback;
const cb_ctx = this.onCompleteCtx;
if (this.store == null and this.system_error != null) {
const system_error = this.system_error.?;
bun.destroy(this);
cb(cb_ctx, ReadFileResultType{ .err = system_error });
return;
} else if (this.store == null) {
bun.destroy(this);
var store = this.store orelse {
if (this.system_error) |system_error| {
bun.destroy(this);
cb(cb_ctx, ReadFileResultType{ .err = system_error });
return;
}
if (Environment.allow_assert) @panic("assertion failure - store should not be null");
bun.destroy(this);
cb(cb_ctx, ReadFileResultType{
.err = SystemError{
.code = bun.String.static("INTERNAL_ERROR"),
@@ -274,12 +275,11 @@ pub const ReadFile = struct {
},
});
return;
}
};
var store = this.store.?;
const buf = this.buffer.items;
defer store.deref();
defer store.deinit();
const system_error = this.system_error;
const total_size = this.total_size;
bun.destroy(this);
@@ -342,8 +342,9 @@ pub const ReadFile = struct {
};
if (this.store) |store| {
if (store.data == .file) {
store.data.file.last_modified = JSC.toJSTime(stat.mtime().sec, stat.mtime().nsec);
const data = &store.getMut().data;
if (data == .file) {
data.file.last_modified = JSC.toJSTime(stat.mtime().sec, stat.mtime().nsec);
}
}
@@ -547,7 +548,7 @@ pub const ReadFileUV = struct {
loop: *libuv.Loop,
file_store: FileStore,
byte_store: ByteStore = ByteStore{ .allocator = bun.default_allocator },
store: *Store,
store: Arc(Store),
offset: SizeType = 0,
max_length: SizeType = Blob.max_size,
total_size: SizeType = Blob.max_size,
@@ -565,7 +566,7 @@ pub const ReadFileUV = struct {
req: libuv.fs_t = std.mem.zeroes(libuv.fs_t),
pub fn start(loop: *libuv.Loop, store: *Store, off: SizeType, max_len: SizeType, comptime Handler: type, handler: *anyopaque) void {
pub fn start(loop: *libuv.Loop, store: Arc(Store), off: SizeType, max_len: SizeType, comptime Handler: type, handler: *anyopaque) void {
log("ReadFileUV.start", .{});
var this = bun.new(ReadFileUV, .{
.loop = loop,
@@ -576,14 +577,13 @@ pub const ReadFileUV = struct {
.on_complete_data = handler,
.on_complete_fn = @ptrCast(&Handler.run),
});
store.ref();
this.getFd(onFileOpen);
}
pub fn finalize(this: *ReadFileUV) void {
log("ReadFileUV.finalize", .{});
defer {
this.store.deref();
this.store.deinit();
this.req.deinit();
bun.destroy(this);
log("ReadFileUV.finalize destroy", .{});
@@ -657,8 +657,9 @@ pub const ReadFileUV = struct {
log("stat: {any}", .{stat});
// keep in sync with resolveSizeAndLastModified
if (this.store.data == .file) {
this.store.data.file.last_modified = JSC.toJSTime(stat.mtime().sec, stat.mtime().nsec);
const data = &this.store.getMut().data;
if (data == .file) {
data.file.last_modified = JSC.toJSTime(stat.mtime().sec, stat.mtime().nsec);
}
if (bun.S.ISDIR(@intCast(stat.mode))) {

View File

@@ -44,6 +44,8 @@ const Syscall = bun.sys;
const uv = bun.windows.libuv;
const AnyBlob = bun.JSC.WebCore.AnyBlob;
const Arc = bun.ptr.Arc;
pub const ReadableStream = struct {
value: JSValue,
ptr: Source,
@@ -339,7 +341,7 @@ pub const ReadableStream = struct {
var store = blob.store orelse {
return ReadableStream.empty(globalThis);
};
switch (store.data) {
switch (store.getMut().data) {
.bytes => {
var reader = ByteBlobLoader.Source.new(
.{
@@ -359,11 +361,10 @@ pub const ReadableStream = struct {
.max_size = if (blob.size != Blob.max_size) blob.size else null,
.lazy = .{
.blob = store,
.blob = store.clone(),
},
},
});
store.ref();
return reader.toReadableStream(globalThis);
},
@@ -373,6 +374,7 @@ pub const ReadableStream = struct {
const proxy = globalThis.bunVM().transpiler.env.getHttpProxy(true, null);
const proxy_url = if (proxy) |p| p.href else null;
// NOTE: contains a reference to blob store field, but store isn't ref'd. This smells like a bug.
return bun.S3.readableStream(credentials, path, blob.offset, if (blob.size != Blob.max_size) blob.size else null, proxy_url, globalThis);
},
}
@@ -387,7 +389,7 @@ pub const ReadableStream = struct {
var store = blob.store orelse {
return ReadableStream.empty(globalThis);
};
switch (store.data) {
switch (store.getMut().data) {
.file => {
var reader = FileReader.Source.new(.{
.globalThis = globalThis,
@@ -395,11 +397,10 @@ pub const ReadableStream = struct {
.event_loop = JSC.EventLoopHandle.init(globalThis.bunVM().eventLoop()),
.start_offset = offset,
.lazy = .{
.blob = store,
.blob = store.clone(),
},
},
});
store.ref();
return reader.toReadableStream(globalThis);
},
@@ -4091,7 +4092,7 @@ pub const FileReader = struct {
pub const Lazy = union(enum) {
none: void,
blob: *Blob.Store,
blob: Arc(Blob.Store),
const OpenedFileBlob = struct {
fd: bun.FileDescriptor,

View File

@@ -97,7 +97,7 @@ pub fn RefCounted(
) type {
const output_name: [:0]const u8 =
if (!bun.Environment.enable_logs)
&""
""
else if (@hasDecl(Context, "debug_name") and Context.debug_name != null)
Context.debug_name
else
@@ -109,14 +109,14 @@ pub fn RefCounted(
const Inner = struct {
/// This is always > 0 unless value is just about to be deallocated.
ref_count: if (intrusive) void else if (sync) AtomicU32 else u32 = if (sync) .init(1) else 1,
ref_count: if (intrusive) void else if (sync) AtomicU32 else u32 = if (intrusive) {} else if (sync) .init(1) else 1,
allocator: Allocator,
value: T,
const Self = @This();
fn ref(this: *Self) callconv(bun.callconv_inline) u32 {
const ref_count_field = @field(if (intrusive) this.value else this, "ref_count");
var ref_count_field = @field(if (intrusive) this.value else this, "ref_count");
if (comptime sync) {
const prev = ref_count_field.fetchAdd(1, AtomicOrder.acquire);
bun.assertWithLocation(prev > 0, @src());
@@ -130,7 +130,7 @@ pub fn RefCounted(
}
fn deref(this: *Self) callconv(bun.callconv_inline) u32 {
const ref_count_field = @field(if (intrusive) this.value else this, "ref_count");
var ref_count_field = @field(if (intrusive) this.value else this, "ref_count");
if (comptime sync) {
const prev = ref_count_field.fetchSub(1, AtomicOrder.release);
bun.assertWithLocation(prev > 0, @src());
@@ -190,14 +190,59 @@ pub fn RefCounted(
return this;
}
pub fn deinit(this: Self) void {
/// Increment the reference count without obtaining a new pointer.
///
/// While this does not return a new pointer, it _does_ create an owned
/// reference. Caller must ensure the new reference's lifecycle is
/// properly managed (that is, its owner calls `.deinit()`), otherwise a
/// memory leak will occur.
pub fn dangerouslyIncrementRef(this: Self) void {
std.mem.doNotOptimizeAway(this.clone());
}
/// Decrement the reference count without deinitializing the pointer.
/// When the reference count hits 0, the value is deinitialized and the
/// allocation is freed.
///
/// ## Safety
/// Decrementing the reference count below 1 while holding a live
/// reference count is illegal behavior. This pointer's owner must die
/// at the same time as this pointer, otherwise it may hold a dangling
/// pointer.
///
/// Caller should set `ensure_alive` to `true` when it assumes the
/// allocation will continue to be alive after this call. This enables
/// runtime safety checks and is useful, for example, when transferring
/// ownership across threads or Zig/c++ boundaries.
pub fn dangerouslyDecrementRef(this: Self, comptime ensure_alive: bool) void {
const prev = this.__ptr.deref();
log("0x{x} deref {d} - 1 = {d}", .{ @intFromPtr(this.__ptr), prev, prev - 1 });
if (prev == 1) {
@branchHint(.unlikely);
if (comptime ensure_alive) bun.assert(false);
// SAFETY: caller ensures that
// 1. owner will die at the same time as this pointer, meaning nobody owns the dangling pointer
// 2. reference count is not decremented below 1, meaning the pointer is not dangling
this.destroy();
}
}
/// Deinitialize the pointer.
///
/// `deinit` decrements the stored allocation's reference count. If that
/// count hits 0, the value gets deinitialized and the allocation is freed.
pub fn deinit(this: *Self) void {
const prev = this.__ptr.deref();
log("0x{x} deref {d} - 1 = {d}", .{ @intFromPtr(this.__ptr), prev, prev - 1 });
if (prev == 1) {
@branchHint(.unlikely);
this.destroy();
}
// pointer cannot be used after .deinit(), even if other references
// to the allocation still exist
this.* = undefined;
}
fn destroy(this: Self) void {
const allocator = this.__ptr.allocator;

View File

@@ -3778,9 +3778,10 @@ pub fn handleTemplateValue(
if (template_value.as(JSC.WebCore.Blob)) |blob| {
if (blob.store) |store| {
if (store.data == .file) {
if (store.data.file.pathlike == .path) {
const path = store.data.file.pathlike.path.slice();
const s = store.get();
if (s.data == .file) {
if (s.data.file.pathlike == .path) {
const path = s.data.file.pathlike.path.slice();
if (!try builder.appendUTF8(path, true)) {
return globalThis.throw("Shell script string contains invalid UTF-16", .{});
}