diff --git a/src/bun.js/api/Shell.classes.ts b/src/bun.js/api/Shell.classes.ts index a86b04e861..570c06584d 100644 --- a/src/bun.js/api/Shell.classes.ts +++ b/src/bun.js/api/Shell.classes.ts @@ -10,6 +10,7 @@ export default [ configurable: false, klass: {}, values: ["resolve", "reject"], + valuesArray: true, proto: { run: { fn: "runFromJS", diff --git a/src/bun.js/api/ShellArgs.classes.ts b/src/bun.js/api/ShellArgs.classes.ts index 5d7b521216..5368ba3a8a 100644 --- a/src/bun.js/api/ShellArgs.classes.ts +++ b/src/bun.js/api/ShellArgs.classes.ts @@ -8,6 +8,7 @@ export default [ finalize: true, hasPendingActivity: false, configurable: false, + valuesArray: true, klass: {}, proto: { setCwd: { diff --git a/src/bun.js/bindings/MarkedArgumentBuffer.zig b/src/bun.js/bindings/MarkedArgumentBuffer.zig index 6b2c3846c8..97e4dbe627 100644 --- a/src/bun.js/bindings/MarkedArgumentBuffer.zig +++ b/src/bun.js/bindings/MarkedArgumentBuffer.zig @@ -8,6 +8,29 @@ pub const MarkedArgumentBuffer = opaque { pub fn run(comptime T: type, ctx: *T, func: *const fn (ctx: *T, args: *MarkedArgumentBuffer) callconv(.c) void) void { MarkedArgumentBuffer__run(@ptrCast(ctx), @ptrCast(func)); } + + pub fn wrap(comptime function: *const fn (globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame, marked_argument_buffer: *MarkedArgumentBuffer) bun.JSError!jsc.JSValue) jsc.JSHostFnZig { + return struct { + pub fn wrapper(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSError!jsc.JSValue { + const Context = struct { + result: bun.JSError!jsc.JSValue, + globalThis: *jsc.JSGlobalObject, + callframe: *jsc.CallFrame, + pub fn run(this: *@This(), marked_argument_buffer: *MarkedArgumentBuffer) callconv(.c) void { + this.result = function(this.globalThis, this.callframe, marked_argument_buffer); + } + }; + + var ctx = Context{ + .globalThis = globalThis, + .callframe = callframe, + .result = undefined, + }; + jsc.MarkedArgumentBuffer.run(Context, &ctx, &Context.run); + return try ctx.result; + } + }.wrapper; + } }; const bun = @import("bun"); diff --git a/src/bun.js/bindings/ShellBindings.cpp b/src/bun.js/bindings/ShellBindings.cpp new file mode 100644 index 0000000000..52a767f66e --- /dev/null +++ b/src/bun.js/bindings/ShellBindings.cpp @@ -0,0 +1,27 @@ +#include "root.h" + +#include "ZigGeneratedClasses.h" + +namespace Bun { + +using namespace JSC; +using namespace WTF; + +extern "C" SYSV_ABI EncodedJSValue Bun__createShellInterpreter(Zig::GlobalObject* _Nonnull globalObject, void* _Nonnull ptr, EncodedJSValue parsed_shell_script, EncodedJSValue resolve, EncodedJSValue reject) +{ + auto& vm = globalObject->vm(); + const auto& existingArgs = jsCast(JSValue::decode(parsed_shell_script))->values(); + WTF::FixedVector> args = WTF::FixedVector>(existingArgs.size()); + for (size_t i = 0; i < existingArgs.size(); i++) { + args[i].setWithoutWriteBarrier(existingArgs[i].get()); + } + JSValue resolveFn = JSValue::decode(resolve); + JSValue rejectFn = JSValue::decode(reject); + auto* structure = globalObject->JSShellInterpreterStructure(); + ASSERT(structure); + + auto* result = WebCore::JSShellInterpreter::create(vm, globalObject, structure, ptr, WTFMove(args), resolveFn, rejectFn); + return JSValue::encode(result); +} + +} diff --git a/src/codegen/class-definitions.ts b/src/codegen/class-definitions.ts index a1792fdb38..7af59d45b0 100644 --- a/src/codegen/class-definitions.ts +++ b/src/codegen/class-definitions.ts @@ -176,6 +176,11 @@ export class ClassDefinition { */ own: Record; values?: string[]; + /** + * When true, the class will accept a MarkedArgumentBuffer* to create a + * WTF::FixedVector jsvalueArray member that will be visited by GC. + */ + valuesArray?: boolean; /** * Set this to `"0b11101110"`. */ diff --git a/src/codegen/generate-classes.ts b/src/codegen/generate-classes.ts index 1215f77d54..85a304578d 100644 --- a/src/codegen/generate-classes.ts +++ b/src/codegen/generate-classes.ts @@ -1429,6 +1429,9 @@ function generateClassHeader(typeName, obj: ClassDefinition) { using Base = JSC::JSDestructibleObject; static constexpr unsigned StructureFlags = Base::StructureFlags${obj.hasOwnProperties() ? ` | HasStaticPropertyTable` : ""}; static ${name}* create(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::Structure* structure, void* ctx); + ${obj.valuesArray ? `static ${name}* create(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::Structure* structure, void* ctx, WTF::FixedVector>&& jsvalueArray);` : ""} + ${obj.valuesArray && obj.values && obj.values.length > 0 ? `static ${name}* create(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::Structure* structure, void* ctx${obj.values.map(v => `, JSC::JSValue ${v}`).join("")});` : ""} + ${obj.valuesArray && obj.values && obj.values.length > 0 ? `static ${name}* create(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::Structure* structure, void* ctx, WTF::FixedVector>&& jsvalueArray${obj.values.map(v => `, JSC::JSValue ${v}`).join("")});` : ""} DECLARE_EXPORT_INFO; template static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) @@ -1465,6 +1468,13 @@ function generateClassHeader(typeName, obj: ClassDefinition) { m_ctx = nullptr; } + ${ + obj.valuesArray + ? `const WTF::FixedVector>& values() const { return jsvalueArray; } + ` + : "" + } + static void analyzeHeap(JSCell*, JSC::HeapAnalyzer&); static ptrdiff_t offsetOfWrapped() { return OBJECT_OFFSETOF(${name}, m_ctx); } @@ -1480,13 +1490,35 @@ function generateClassHeader(typeName, obj: ClassDefinition) { void* m_ctx { nullptr }; - ${name}(JSC::VM& vm, JSC::Structure* structure, void* sinkPtr) - : Base(vm, structure) + ${name}(JSC::VM& vm, JSC::Structure* structure, void* sinkPtr${obj.valuesArray ? ", WTF::FixedVector>&& jsvalueArray_" : ""}) + : Base(vm, structure)${obj.valuesArray ? ", jsvalueArray(WTFMove(jsvalueArray_))" : ""} { m_ctx = sinkPtr; ${weakInit.trim()} } + ${ + obj.valuesArray && obj.values && obj.values.length > 0 + ? `${name}(JSC::VM& vm, JSC::Structure* structure, void* sinkPtr${obj.values.map(v => `, JSC::JSValue ${v}`).join("")}) + : Base(vm, structure)${obj.values.map(v => `\n , m_${v}(${v}, JSC::WriteBarrierEarlyInit)`).join("")} + { + m_ctx = sinkPtr; + ${weakInit.trim()} + }` + : "" + } + + ${ + obj.valuesArray && obj.values && obj.values.length > 0 + ? `${name}(JSC::VM& vm, JSC::Structure* structure, void* sinkPtr, WTF::FixedVector>&& jsvalueArray_${obj.values.map(v => `, JSC::JSValue ${v}`).join("")}) + : Base(vm, structure), jsvalueArray(WTFMove(jsvalueArray_))${obj.values.map(v => `\n , m_${v}(${v}, JSC::WriteBarrierEarlyInit)`).join("")} + { + m_ctx = sinkPtr; + ${weakInit.trim()} + }` + : "" + } + void finishCreation(JSC::VM&); ${Object.entries(obj.custom ?? {}) @@ -1512,6 +1544,7 @@ function generateClassHeader(typeName, obj: ClassDefinition) { ${renderCachedFieldsHeader(typeName, klass, proto, values)} ${callbacks ? renderCallbacksHeader(typeName, obj.callbacks) : ""} + ${obj.valuesArray ? "WTF::FixedVector> jsvalueArray;" : ""} }; ${suffix} `.trim(); @@ -1594,6 +1627,7 @@ void ${name}::visitAdditionalChildren(Visitor& visitor) ASSERT_GC_OBJECT_INHERITS(thisObject, info()); ${values} ${DEFINE_VISIT_CHILDREN_LIST} + ${obj.valuesArray ? "for (auto& value : thisObject->jsvalueArray) { visitor.append(value); }" : ""} ${hasPendingActivity ? "visitor.addOpaqueRoot(this->wrapped());" : ""} } @@ -1727,11 +1761,41 @@ void ${name}::finishCreation(VM& vm) } ${name}* ${name}::create(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::Structure* structure, void* ctx) { - ${name}* ptr = new (NotNull, JSC::allocateCell<${name}>(vm)) ${name}(vm, structure, ctx); + ${name}* ptr = new (NotNull, JSC::allocateCell<${name}>(vm)) ${name}(vm, structure, ctx${obj.valuesArray ? ", WTF::FixedVector>()" : ""}); ptr->finishCreation(vm); return ptr; } +${ + obj.valuesArray + ? `${name}* ${name}::create(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::Structure* structure, void* ctx, WTF::FixedVector>&& jsvalueArray) { + ${name}* ptr = new (NotNull, JSC::allocateCell<${name}>(vm)) ${name}(vm, structure, ctx, WTFMove(jsvalueArray)); + ptr->finishCreation(vm); + return ptr; +}` + : "" +} + +${ + obj.valuesArray && obj.values && obj.values.length > 0 + ? `${name}* ${name}::create(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::Structure* structure, void* ctx${obj.values.map(v => `, JSC::JSValue ${v}`).join("")}) { + ${name}* ptr = new (NotNull, JSC::allocateCell<${name}>(vm)) ${name}(vm, structure, ctx${obj.values.map(v => `, ${v}`).join("")}); + ptr->finishCreation(vm); + return ptr; +}` + : "" +} + +${ + obj.valuesArray && obj.values && obj.values.length > 0 + ? `${name}* ${name}::create(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::Structure* structure, void* ctx, WTF::FixedVector>&& jsvalueArray${obj.values.map(v => `, JSC::JSValue ${v}`).join("")}) { + ${name}* ptr = new (NotNull, JSC::allocateCell<${name}>(vm)) ${name}(vm, structure, ctx, WTFMove(jsvalueArray)${obj.values.map(v => `, ${v}`).join("")}); + ptr->finishCreation(vm); + return ptr; +}` + : "" +} + extern JSC_CALLCONV void* JSC_HOST_CALL_ATTRIBUTES ${typeName}__fromJS(JSC::EncodedJSValue value) { JSC::JSValue decodedValue = JSC::JSValue::decode(value); if (decodedValue.isEmpty() || !decodedValue.isCell()) @@ -1831,6 +1895,70 @@ extern JSC_CALLCONV JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES ${typeName}__cr return JSValue::encode(instance); } +${ + obj.valuesArray + ? `extern JSC_CALLCONV JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES ${typeName}__createWithValues(Zig::GlobalObject* globalObject, void* ptr, void* markedArgumentBuffer) { + auto &vm = globalObject->vm(); + JSC::Structure* structure = globalObject->${className(typeName)}Structure(); + auto* args = static_cast(markedArgumentBuffer); + WTF::FixedVector> jsvalueArray(args->size()); + for (size_t i = 0; i < args->size(); ++i) { + jsvalueArray[i].setWithoutWriteBarrier(args->at(i)); + } + ${className(typeName)}* instance = ${className(typeName)}::create(vm, globalObject, structure, ptr, WTFMove(jsvalueArray)); + ${ + obj.estimatedSize + ? ` + auto size = ${symbolName(typeName, "estimatedSize")}(ptr); + vm.heap.reportExtraMemoryAllocated(instance, size);` + : "" + } + return JSValue::encode(instance); +}` + : "" +} + +${ + obj.valuesArray && obj.values && obj.values.length > 0 + ? `extern JSC_CALLCONV JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES ${typeName}__createWithInitialValues(Zig::GlobalObject* globalObject, void* ptr${obj.values.map(v => `, JSC::EncodedJSValue ${v}`).join("")}) { + auto &vm = globalObject->vm(); + JSC::Structure* structure = globalObject->${className(typeName)}Structure(); + ${className(typeName)}* instance = ${className(typeName)}::create(vm, globalObject, structure, ptr${obj.values.map(v => `, JSC::JSValue::decode(${v})`).join("")}); + ${ + obj.estimatedSize + ? ` + auto size = ${symbolName(typeName, "estimatedSize")}(ptr); + vm.heap.reportExtraMemoryAllocated(instance, size);` + : "" + } + return JSValue::encode(instance); +}` + : "" +} + +${ + obj.valuesArray && obj.values && obj.values.length > 0 + ? `extern JSC_CALLCONV JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES ${typeName}__createWithValuesAndInitialValues(Zig::GlobalObject* globalObject, void* ptr, void* markedArgumentBuffer${obj.values.map(v => `, JSC::EncodedJSValue ${v}`).join("")}) { + auto &vm = globalObject->vm(); + JSC::Structure* structure = globalObject->${className(typeName)}Structure(); + auto* args = static_cast(markedArgumentBuffer); + WTF::FixedVector> jsvalueArray(args->size()); + for (size_t i = 0; i < args->size(); ++i) { + jsvalueArray[i].setWithoutWriteBarrier(args->at(i)); + } + ${className(typeName)}* instance = ${className(typeName)}::create(vm, globalObject, structure, ptr, WTFMove(jsvalueArray)${obj.values.map(v => `, JSC::JSValue::decode(${v})`).join("")}); + ${ + obj.estimatedSize + ? ` + auto size = ${symbolName(typeName, "estimatedSize")}(ptr); + vm.heap.reportExtraMemoryAllocated(instance, size);` + : "" + } + return JSValue::encode(instance); +}` + : "" +} + ${DEFINE_VISIT_CHILDREN} @@ -1891,6 +2019,7 @@ function generateZig( call = false, memoryCost, values = [], + valuesArray = false, hasPendingActivity = false, structuredClone = false, getInternalProperties = false, @@ -2316,6 +2445,57 @@ pub const ${className(typeName)} = struct { : "" } + ${ + valuesArray && !overridesToJS + ? ` + /// Create a new instance of ${typeName} with a MarkedArgumentBuffer + pub fn toJSWithValues(this: *${typeName}, globalObject: *jsc.JSGlobalObject, markedArgumentBuffer: *jsc.MarkedArgumentBuffer) jsc.JSValue { + if (comptime Environment.enable_logs) log_zig_to_js("${typeName}"); + if (comptime Environment.allow_assert) { + const value__ = ${symbolName(typeName, "createWithValues")}(globalObject, this, markedArgumentBuffer); + @import("bun").assert(value__.as(${typeName}).? == this); // If this fails, likely a C ABI issue. + return value__; + } else { + return ${symbolName(typeName, "createWithValues")}(globalObject, this, markedArgumentBuffer); + } + }` + : "" + } + + ${ + valuesArray && values.length > 0 && !overridesToJS + ? ` + /// Create a new instance of ${typeName} with initial values + pub fn toJSWithInitialValues(this: *${typeName}, globalObject: *jsc.JSGlobalObject${values.map(v => `, ${v}: jsc.JSValue`).join("")}) jsc.JSValue { + if (comptime Environment.enable_logs) log_zig_to_js("${typeName}"); + if (comptime Environment.allow_assert) { + const value__ = ${symbolName(typeName, "createWithInitialValues")}(globalObject, this${values.map(v => `, ${v}`).join("")}); + @import("bun").assert(value__.as(${typeName}).? == this); // If this fails, likely a C ABI issue. + return value__; + } else { + return ${symbolName(typeName, "createWithInitialValues")}(globalObject, this${values.map(v => `, ${v}`).join("")}); + } + }` + : "" + } + + ${ + valuesArray && values.length > 0 && !overridesToJS + ? ` + /// Create a new instance of ${typeName} with both a MarkedArgumentBuffer and initial values + pub fn toJSWithValuesAndInitialValues(this: *${typeName}, globalObject: *jsc.JSGlobalObject, markedArgumentBuffer: *jsc.MarkedArgumentBuffer${values.map(v => `, ${v}: jsc.JSValue`).join("")}) jsc.JSValue { + if (comptime Environment.enable_logs) log_zig_to_js("${typeName}"); + if (comptime Environment.allow_assert) { + const value__ = ${symbolName(typeName, "createWithValuesAndInitialValues")}(globalObject, this, markedArgumentBuffer${values.map(v => `, ${v}`).join("")}); + @import("bun").assert(value__.as(${typeName}).? == this); // If this fails, likely a C ABI issue. + return value__; + } else { + return ${symbolName(typeName, "createWithValuesAndInitialValues")}(globalObject, this, markedArgumentBuffer${values.map(v => `, ${v}`).join("")}); + } + }` + : "" + } + /// Modify the internal ptr to point to a new instance of ${typeName}. pub fn dangerouslySetPtr(value: jsc.JSValue, ptr: ?*${typeName}) bool { jsc.markBinding(@src()); @@ -2332,6 +2512,9 @@ pub const ${className(typeName)} = struct { extern fn ${symbolName(typeName, "fromJSDirect")}(jsc.JSValue) callconv(jsc.conv) ?*${typeName}; extern fn ${symbolName(typeName, "getConstructor")}(*jsc.JSGlobalObject) callconv(jsc.conv) jsc.JSValue; extern fn ${symbolName(typeName, "create")}(globalObject: *jsc.JSGlobalObject, ptr: ?*${typeName}) callconv(jsc.conv) jsc.JSValue; + ${valuesArray ? `extern fn ${symbolName(typeName, "createWithValues")}(globalObject: *jsc.JSGlobalObject, ptr: ?*${typeName}, markedArgumentBuffer: *anyopaque) callconv(jsc.conv) jsc.JSValue;` : ""} + ${valuesArray && values.length > 0 ? `extern fn ${symbolName(typeName, "createWithInitialValues")}(globalObject: *jsc.JSGlobalObject, ptr: ?*${typeName}${values.map(v => `, ${v}: jsc.JSValue`).join("")}) callconv(jsc.conv) jsc.JSValue;` : ""} + ${valuesArray && values.length > 0 ? `extern fn ${symbolName(typeName, "createWithValuesAndInitialValues")}(globalObject: *jsc.JSGlobalObject, ptr: ?*${typeName}, markedArgumentBuffer: *anyopaque${values.map(v => `, ${v}: jsc.JSValue`).join("")}) callconv(jsc.conv) jsc.JSValue;` : ""} /// Create a new instance of ${typeName} without validating it works. pub const toJSUnchecked = ${symbolName(typeName, "create")}; diff --git a/src/shell/ParsedShellScript.zig b/src/shell/ParsedShellScript.zig index f0aff100d0..ee7b3be1c6 100644 --- a/src/shell/ParsedShellScript.zig +++ b/src/shell/ParsedShellScript.zig @@ -41,9 +41,6 @@ pub fn finalize( if (this.export_env) |*env| env.deinit(); if (this.cwd) |*cwd| cwd.deref(); - for (this.jsobjs.items) |jsobj| { - jsobj.unprotect(); - } if (this.args) |a| a.deinit(); bun.destroy(this); } @@ -102,8 +99,12 @@ pub fn setEnv(this: *ParsedShellScript, globalThis: *JSGlobalObject, callframe: return .js_undefined; } -pub fn createParsedShellScript(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun.JSError!JSValue { +pub const createParsedShellScript = jsc.MarkedArgumentBuffer.wrap(createParsedShellScriptImpl); + +fn createParsedShellScriptImpl(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame, marked_argument_buffer: *jsc.MarkedArgumentBuffer) bun.JSError!JSValue { var shargs = ShellArgs.init(); + var needs_to_free_shargs = true; + defer if (needs_to_free_shargs) shargs.deinit(); const arguments_ = callframe.arguments_old(2); const arguments = arguments_.slice(); @@ -124,7 +125,7 @@ pub fn createParsedShellScript(globalThis: *jsc.JSGlobalObject, callframe: *jsc. } var jsobjs = std.ArrayList(JSValue).init(shargs.arena_allocator()); var script = std.ArrayList(u8).init(shargs.arena_allocator()); - try bun.shell.shellCmdFromJS(globalThis, string_args, &template_args, &jsobjs, &jsstrings, &script); + try bun.shell.shellCmdFromJS(globalThis, string_args, &template_args, &jsobjs, &jsstrings, &script, marked_argument_buffer); var parser: ?bun.shell.Parser = null; var lex_result: ?shell.LexResult = null; @@ -159,9 +160,12 @@ pub fn createParsedShellScript(globalThis: *jsc.JSGlobalObject, callframe: *jsc. .args = shargs, .jsobjs = jsobjs, }); - parsed_shell_script.this_jsvalue = jsc.Codegen.JSParsedShellScript.toJS(parsed_shell_script, globalThis); + const this_jsvalue = jsc.Codegen.JSParsedShellScript.toJSWithValues(parsed_shell_script, globalThis, marked_argument_buffer); + parsed_shell_script.this_jsvalue = this_jsvalue; + bun.analytics.Features.shell += 1; - return parsed_shell_script.this_jsvalue; + needs_to_free_shargs = false; + return this_jsvalue; } const std = @import("std"); diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig index 04fe05f620..6a5efa110c 100644 --- a/src/shell/interpreter.zig +++ b/src/shell/interpreter.zig @@ -707,16 +707,23 @@ pub const Interpreter = struct { interpreter.flags.quiet = quiet; interpreter.globalThis = globalThis; - const js_value = jsc.Codegen.JSShellInterpreter.toJS(interpreter, globalThis); + const js_value = Bun__createShellInterpreter( + globalThis, + interpreter, + parsed_shell_script_js, + resolve, + reject, + ); interpreter.this_jsvalue = js_value; - jsc.Codegen.JSShellInterpreter.resolveSetCached(js_value, globalThis, resolve); - jsc.Codegen.JSShellInterpreter.rejectSetCached(js_value, globalThis, reject); + interpreter.keep_alive.ref(globalThis.bunVM()); bun.analytics.Features.shell += 1; return js_value; } + extern fn Bun__createShellInterpreter(globalThis: *jsc.JSGlobalObject, ptr: *Interpreter, parsed_shell_script: JSValue, resolve: JSValue, reject: JSValue) callconv(jsc.conv) JSValue; + pub fn parse( arena_allocator: std.mem.Allocator, script: []const u8, @@ -1170,9 +1177,6 @@ pub const Interpreter = struct { fn deinitAfterJSRun(this: *ThisInterpreter) void { log("Interpreter(0x{x}) deinitAfterJSRun", .{@intFromPtr(this)}); - for (this.jsobjs) |jsobj| { - jsobj.unprotect(); - } this.root_io.deref(); this.keep_alive.disable(); this.root_shell.deinitImpl(false, false); @@ -1192,9 +1196,6 @@ pub const Interpreter = struct { fn deinitEverything(this: *ThisInterpreter) void { log("deinit interpreter", .{}); - for (this.jsobjs) |jsobj| { - jsobj.unprotect(); - } this.root_io.deref(); this.root_shell.deinitImpl(false, true); for (this.vm_args_utf8.items[0..]) |str| { diff --git a/src/shell/shell.zig b/src/shell/shell.zig index 5c3883f05d..e4110869a9 100644 --- a/src/shell/shell.zig +++ b/src/shell/shell.zig @@ -3705,6 +3705,7 @@ pub fn shellCmdFromJS( out_jsobjs: *std.ArrayList(JSValue), jsstrings: *std.ArrayList(bun.String), out_script: *std.ArrayList(u8), + marked_argument_buffer: *jsc.MarkedArgumentBuffer, ) bun.JSError!void { var builder = ShellSrcBuilder.init(globalThis, out_script, jsstrings); var jsobjref_buf: [128]u8 = [_]u8{0} ** 128; @@ -3723,7 +3724,7 @@ pub fn shellCmdFromJS( const template_value = try template_args.next() orelse { return globalThis.throw("Shell script is missing JSValue arg", .{}); }; - try handleTemplateValue(globalThis, template_value, out_jsobjs, out_script, jsstrings, jsobjref_buf[0..]); + try handleTemplateValue(globalThis, template_value, out_jsobjs, out_script, jsstrings, jsobjref_buf[0..], marked_argument_buffer); } } return; @@ -3736,13 +3737,14 @@ pub fn handleTemplateValue( out_script: *std.ArrayList(u8), jsstrings: *std.ArrayList(bun.String), jsobjref_buf: []u8, + marked_argument_buffer: *jsc.MarkedArgumentBuffer, ) bun.JSError!void { var builder = ShellSrcBuilder.init(globalThis, out_script, jsstrings); if (template_value != .zero) { if (template_value.asArrayBuffer(globalThis)) |array_buffer| { _ = array_buffer; const idx = out_jsobjs.items.len; - template_value.protect(); + marked_argument_buffer.append(template_value); try out_jsobjs.append(template_value); const slice = std.fmt.bufPrint(jsobjref_buf[0..], "{s}{d}", .{ LEX_JS_OBJREF_PREFIX, idx }) catch return globalThis.throwOutOfMemory(); try out_script.appendSlice(slice); @@ -3763,7 +3765,7 @@ pub fn handleTemplateValue( } const idx = out_jsobjs.items.len; - template_value.protect(); + marked_argument_buffer.append(template_value); try out_jsobjs.append(template_value); const slice = std.fmt.bufPrint(jsobjref_buf[0..], "{s}{d}", .{ LEX_JS_OBJREF_PREFIX, idx }) catch return globalThis.throwOutOfMemory(); try out_script.appendSlice(slice); @@ -3774,7 +3776,7 @@ pub fn handleTemplateValue( _ = rstream; const idx = out_jsobjs.items.len; - template_value.protect(); + marked_argument_buffer.append(template_value); try out_jsobjs.append(template_value); const slice = std.fmt.bufPrint(jsobjref_buf[0..], "{s}{d}", .{ LEX_JS_OBJREF_PREFIX, idx }) catch return globalThis.throwOutOfMemory(); try out_script.appendSlice(slice); @@ -3785,7 +3787,7 @@ pub fn handleTemplateValue( _ = req; const idx = out_jsobjs.items.len; - template_value.protect(); + marked_argument_buffer.append(template_value); try out_jsobjs.append(template_value); const slice = std.fmt.bufPrint(jsobjref_buf[0..], "{s}{d}", .{ LEX_JS_OBJREF_PREFIX, idx }) catch return globalThis.throwOutOfMemory(); try out_script.appendSlice(slice); @@ -3804,7 +3806,7 @@ pub fn handleTemplateValue( const last = array.len -| 1; var i: u32 = 0; while (try array.next()) |arr| : (i += 1) { - try handleTemplateValue(globalThis, arr, out_jsobjs, out_script, jsstrings, jsobjref_buf); + try handleTemplateValue(globalThis, arr, out_jsobjs, out_script, jsstrings, jsobjref_buf, marked_argument_buffer); if (i < last) { const str = bun.String.static(" "); if (!try builder.appendBunStr(str, false)) { @@ -4299,9 +4301,12 @@ pub const TestingAPIs = struct { return .false; } - pub fn shellLex( + pub const shellLex = jsc.MarkedArgumentBuffer.wrap(shellLexImpl); + + fn shellLexImpl( globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame, + marked_argument_buffer: *jsc.MarkedArgumentBuffer, ) bun.JSError!jsc.JSValue { const arguments_ = callframe.arguments_old(2); var arguments = jsc.CallFrame.ArgumentsSlice.init(globalThis.bunVM(), arguments_.slice()); @@ -4325,14 +4330,10 @@ pub const TestingAPIs = struct { jsstrings.deinit(); } var jsobjs = std.ArrayList(JSValue).init(arena.allocator()); - defer { - for (jsobjs.items) |jsval| { - jsval.unprotect(); - } - } + defer jsobjs.deinit(); var script = std.ArrayList(u8).init(arena.allocator()); - try shellCmdFromJS(globalThis, string_args, &template_args, &jsobjs, &jsstrings, &script); + try shellCmdFromJS(globalThis, string_args, &template_args, &jsobjs, &jsstrings, &script, marked_argument_buffer); const lex_result = brk: { if (bun.strings.isAllASCII(script.items[0..])) { @@ -4367,9 +4368,12 @@ pub const TestingAPIs = struct { return bun_str.toJS(globalThis); } - pub fn shellParse( + pub const shellParse = jsc.MarkedArgumentBuffer.wrap(shellParseImpl); + + fn shellParseImpl( globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame, + marked_argument_buffer: *jsc.MarkedArgumentBuffer, ) bun.JSError!jsc.JSValue { const arguments_ = callframe.arguments_old(2); var arguments = jsc.CallFrame.ArgumentsSlice.init(globalThis.bunVM(), arguments_.slice()); @@ -4393,13 +4397,9 @@ pub const TestingAPIs = struct { jsstrings.deinit(); } var jsobjs = std.ArrayList(JSValue).init(arena.allocator()); - defer { - for (jsobjs.items) |jsval| { - jsval.unprotect(); - } - } + defer jsobjs.deinit(); var script = std.ArrayList(u8).init(arena.allocator()); - try shellCmdFromJS(globalThis, string_args, &template_args, &jsobjs, &jsstrings, &script); + try shellCmdFromJS(globalThis, string_args, &template_args, &jsobjs, &jsstrings, &script, marked_argument_buffer); var out_parser: ?Parser = null; var out_lex_result: ?LexResult = null; @@ -4422,8 +4422,7 @@ pub const TestingAPIs = struct { const str = try std.json.stringifyAlloc(globalThis.bunVM().allocator, script_ast, .{}); defer globalThis.bunVM().allocator.free(str); - var bun_str = bun.String.fromBytes(str); - return bun_str.toJS(globalThis); + return bun.String.createUTF8ForJS(globalThis, str); } }; diff --git a/test/js/bun/shell/shell-leak-args.test.ts b/test/js/bun/shell/shell-leak-args.test.ts new file mode 100644 index 0000000000..cd05212788 --- /dev/null +++ b/test/js/bun/shell/shell-leak-args.test.ts @@ -0,0 +1,26 @@ +import { $ } from "bun"; +import { expect, test } from "bun:test"; + +test("shell parsing error does not leak emmory", async () => { + const buffer = Buffer.alloc(1024 * 1024, "A").toString(); + for (let i = 0; i < 5; i++) { + try { + $`${{ raw: buffer }} `; + } catch (e) {} + } + const rss = process.memoryUsage.rss(); + for (let i = 0; i < 200; i++) { + try { + $`${{ raw: buffer }} `; + } catch (e) {} + } + const after = process.memoryUsage.rss() / 1024 / 1024; + const before = rss / 1024 / 1024; + // In Bun v1.3.0 on macOS arm64: + // Expected: < 100 + // Received: 524.65625 + // In Bun v1.3.1 on macOS arm64: + // Expected: < 100 + // Received: 0.25 + expect(after - before).toBeLessThan(100); +});