Merge branch 'main' of https://github.com/oven-sh/bun into don/fix/test-label-uaf

src/bun.js/base.zig

@@ -239,6 +239,7 @@ pub fn toInvalidArguments(
     return JSC.Error.ERR_INVALID_ARG_TYPE.fmt(ctx, fmt, args);
 }
 
+/// Deprecated. Use `bun.default_allocator` instead.
 pub fn getAllocator(_: js.JSContextRef) std.mem.Allocator {
     return default_allocator;
 }

src/bun.js/test/expect.zig

@@ -378,10 +378,13 @@ pub const Expect = struct {
             .custom_label = custom_label,
             .parent = if (Jest.runner) |runner|
                 if (runner.pending_test) |pending|
-                    Expect.ParentScope{ .TestScope = Expect.TestScope{
-                        .describe = pending.describe,
-                        .test_id = pending.test_id,
-                    } }
+                    Expect.ParentScope{
+                        .TestScope = Expect.TestScope{
+                            // FIXME: Expect obtains a DescribeScope reference without managing its count.
+                            .describe = pending.describe.get(),
+                            .test_id = pending.test_id,
+                        },
+                    }
                 else
                     Expect.ParentScope{ .global = {} }
             else

src/bun.js/test/jest.zig

@@ -7,6 +7,7 @@ const MimeType = bun.http.MimeType;
 const ZigURL = @import("../../url.zig").URL;
 const HTTPClient = bun.http;
 const Environment = bun.Environment;
+const Allocator = std.mem.Allocator;
 
 const Snapshots = @import("./snapshot.zig").Snapshots;
 const expect = @import("./expect.zig");
@@ -67,7 +68,7 @@ pub const TestRunner = struct {
     last_file: u64 = 0,
     bail: u32 = 0,
 
-    allocator: std.mem.Allocator,
+    allocator: Allocator,
     callback: *Callback = undefined,
 
     drainer: JSC.AnyTask = undefined,
@@ -557,11 +558,11 @@ pub const TestScope = struct {
         actual: u32 = 0,
     };
 
-    pub fn deinit(this: *TestScope, globalThis: *JSGlobalObject) void {
+    pub fn deinit(this: *TestScope) void {
         if (this.label.len > 0) {
             const label = this.label;
             this.label = "";
-            getAllocator(globalThis).free(label);
+            bun.default_allocator.free(label);
         }
     }
 
@@ -811,8 +812,11 @@ pub const DescribeScope = struct {
     value: JSValue = .zero,
     done: bool = false,
     skip_count: u32 = 0,
+    ref_count: u32 = 1,
     tag: Tag = .pass,
 
+    pub usingnamespace bun.NewRefCounted(@This(), deinit, "DescribeScope");
+
     fn isWithinOnlyScope(this: *const DescribeScope) bool {
         if (this.tag == .only) return true;
         if (this.parent != null) return this.parent.?.isWithinOnlyScope();
@@ -888,6 +892,24 @@ pub const DescribeScope = struct {
         }.run;
     }
 
+    fn enqueueNewTask(
+        this: *DescribeScope,
+        globalObject: *JSGlobalObject,
+        test_id: TestRunner.Test.ID,
+        source: *const logger.Source,
+        test_id_for_debugger: ?TestRunner.Test.ID,
+    ) void {
+        var task = bun.new(TestRunnerTask, .{
+            .test_id = test_id,
+            .describe = this.refPtr(),
+            .globalThis = globalObject,
+            .source_file_path = source.path.text,
+            .test_id_for_debugger = test_id_for_debugger orelse 0,
+        });
+        task.ref.ref(globalObject.bunVM());
+        Jest.runner.?.enqueue(task);
+    }
+
     pub fn onDone(
         ctx: js.JSContextRef,
         callframe: *CallFrame,
@@ -1117,7 +1139,7 @@ pub const DescribeScope = struct {
         globalObject.clearTerminationException();
 
         const file = this.file_id;
-        const allocator = getAllocator(globalObject);
+        const allocator = bun.default_allocator;
         const tests: []TestScope = this.tests.items;
         const end = @as(TestRunner.Test.ID, @truncate(tests.len));
         this.pending_tests = std.DynamicBitSetUnmanaged.initFull(allocator, end) catch unreachable;
@@ -1136,21 +1158,16 @@ pub const DescribeScope = struct {
                     Jest.runner.?.reportFailure(i + this.test_id_start, source.path.text, tests[i].label, 0, 0, this);
                     i += 1;
                 }
-                this.deinit(globalObject);
+                this.deref();
                 return;
             }
             if (end == 0) {
-                var runner = allocator.create(TestRunnerTask) catch unreachable;
-                runner.* = .{
-                    .test_id = std.math.maxInt(TestRunner.Test.ID),
-                    .describe = this,
-                    .globalThis = globalObject,
-                    .source_file_path = source.path.text,
-                    .test_id_for_debugger = 0,
-                };
-                runner.ref.ref(globalObject.bunVM());
-
-                Jest.runner.?.enqueue(runner);
+                this.enqueueNewTask(
+                    globalObject,
+                    std.math.maxInt(TestRunner.Test.ID),
+                    &source,
+                    null,
+                );
                 return;
             }
         }
@@ -1158,17 +1175,12 @@ pub const DescribeScope = struct {
         const maybe_report_debugger = max_test_id_for_debugger > 0;
 
         while (i < end) : (i += 1) {
-            var runner = allocator.create(TestRunnerTask) catch unreachable;
-            runner.* = .{
-                .test_id = i,
-                .describe = this,
-                .globalThis = globalObject,
-                .source_file_path = source.path.text,
-                .test_id_for_debugger = if (maybe_report_debugger) tests[i].test_id_for_debugger else 0,
-            };
-            runner.ref.ref(globalObject.bunVM());
-
-            Jest.runner.?.enqueue(runner);
+            this.enqueueNewTask(
+                globalObject,
+                i,
+                &source,
+                if (maybe_report_debugger) tests[i].test_id_for_debugger else 0,
+            );
         }
     }
 
@@ -1195,11 +1207,11 @@ pub const DescribeScope = struct {
                 _ = globalThis.bunVM().uncaughtException(globalThis, err, true);
             }
         }
-        this.deinit(globalThis);
+        this.deref();
     }
 
-    pub fn deinit(this: *DescribeScope, globalThis: *JSGlobalObject) void {
-        const allocator = getAllocator(globalThis);
+    fn deinit(this: *DescribeScope) void {
+        const allocator = bun.default_allocator;
 
         if (this.label.len > 0) {
             const label = this.label;
@@ -1209,7 +1221,7 @@ pub const DescribeScope = struct {
 
         this.pending_tests.deinit(allocator);
         for (this.tests.items) |*t| {
-            t.deinit(globalThis);
+            t.deinit();
         }
         this.tests.clearAndFree(allocator);
     }
@@ -1259,7 +1271,7 @@ pub const WrappedDescribeScope = struct {
 pub const TestRunnerTask = struct {
     test_id: TestRunner.Test.ID,
     test_id_for_debugger: TestRunner.Test.ID,
-    describe: *DescribeScope,
+    describe: DescribeScope.Ptr,
     globalThis: *JSGlobalObject,
     source_file_path: string = "",
     needs_before_each: bool = true,
@@ -1331,10 +1343,13 @@ pub const TestRunnerTask = struct {
     }
 
     pub fn run(this: *TestRunnerTask) bool {
-        var describe = this.describe;
+        var describe = this.describe.get();
         var globalThis = this.globalThis;
         var jsc_vm = globalThis.bunVM();
 
+        describe.ref();
+        defer describe.deref();
+
         // reset the global state for each test
         // prior to the run
         expect.active_test_expectation_counter = .{};
@@ -1350,7 +1365,7 @@ pub const TestRunnerTask = struct {
             return false;
         }
 
-        var test_: TestScope = this.describe.tests.items[test_id];
+        var test_: TestScope = describe.tests.items[test_id];
         describe.current_test_id = test_id;
         const test_id_for_debugger = test_.test_id_for_debugger;
         this.test_id_for_debugger = test_id_for_debugger;
@@ -1377,9 +1392,9 @@ pub const TestRunnerTask = struct {
             this.needs_before_each = false;
             const label = test_.label;
 
-            if (this.describe.runCallback(globalThis, .beforeEach)) |err| {
+            if (describe.runCallback(globalThis, .beforeEach)) |err| {
                 _ = jsc_vm.uncaughtException(globalThis, err, true);
-                Jest.runner.?.reportFailure(test_id, this.source_file_path, label, 0, 0, this.describe);
+                Jest.runner.?.reportFailure(test_id, this.source_file_path, label, 0, 0, describe);
                 return false;
             }
         }
@@ -1388,8 +1403,8 @@ pub const TestRunnerTask = struct {
         jsc_vm.auto_killer.enable();
         var result = TestScope.run(&test_, this);
 
-        if (this.describe.tests.items.len > test_id) {
-            this.describe.tests.items[test_id].timeout_millis = test_.timeout_millis;
+        if (describe.tests.items.len > test_id) {
+            describe.tests.items[test_id].timeout_millis = test_.timeout_millis;
         }
 
         // rejected promises should fail the test
@@ -1516,12 +1531,12 @@ pub const TestRunnerTask = struct {
         this.reported = true;
 
         const test_id = this.test_id;
-        var test_ = this.describe.tests.items[test_id];
+        var test_ = this.describe.get().tests.items[test_id];
         if (from == .timeout) {
             test_.timeout_millis = @truncate(from.timeout);
         }
 
-        var describe = this.describe;
+        var describe = this.describe.get();
         describe.tests.items[test_id] = test_;
 
         if (from == .timeout) {
@@ -1657,6 +1672,7 @@ pub const TestRunnerTask = struct {
         //
         // TODO: fix this bug
         // default_allocator.destroy(this);
+        // this.describe.deinit();
     }
 };
 
@@ -2043,7 +2059,7 @@ fn eachBind(globalThis: *JSGlobalObject, callframe: *CallFrame) bun.JSError!JSVa
     const parent = DescribeScope.active.?;
 
     if (JSC.getFunctionData(callee)) |data| {
-        const allocator = getAllocator(globalThis);
+        const allocator = bun.default_allocator;
         const each_data = bun.cast(*EachData, data);
         JSC.setFunctionData(callee, null);
         const array = each_data.*.strong.get() orelse return .undefined;
@@ -2146,13 +2162,24 @@ fn eachBind(globalThis: *JSGlobalObject, callframe: *CallFrame) bun.JSError!JSVa
                     }) catch unreachable;
                 }
             } else {
-                var scope = allocator.create(DescribeScope) catch unreachable;
-                scope.* = .{
+                var scope = DescribeScope.new(.{
                     .label = formattedLabel,
                     .parent = parent,
                     .file_id = parent.file_id,
                     .tag = tag,
-                };
+                });
+                // scope.* = .{
+                //     .label = formattedLabel,
+                //     .parent = parent,
+                //     .file_id = parent.file_id,
+                //     .tag = tag,
+                // };
+                // var scope = DescribeScope.Ptr.new(.{
+                //     .label = formattedLabel,
+                //     .parent = parent,
+                //     .file_id = parent.file_id,
+                //     .tag = tag,
+                // });
 
                 const ret = scope.run(globalThis, function, function_args);
                 _ = ret;

src/cli/test_command.zig

@@ -1729,6 +1729,13 @@ pub const TestCommand = struct {
 
                 vm.onUnhandledRejectionCtx = null;
                 vm.onUnhandledRejection = jest.TestRunnerTask.onUnhandledRejection;
+                if (repeat_count > 1) {
+                    // subsequent runs will try to use now-free'd memory. Since
+                    // this feature is used so infrequently, we'll just leak it
+                    // lol
+                    @branchHint(.unlikely);
+                    module.ref();
+                }
                 module.runTests(vm.global);
                 vm.eventLoop().tick();
 

src/ptr/ref_count.zig

@@ -47,7 +47,9 @@ pub fn NewRefCounted(comptime T: type, comptime deinit_fn: ?fn (self: *T) void,
                 }
             }
 
-            if (bun.Environment.isDebug) log("0x{x} deref {d} - 1 = {d}", .{ @intFromPtr(self), ref_count, ref_count - 1 });
+            if (bun.Environment.isDebug) {
+                log("0x{x} deref {d} - 1 = {d}", .{ @intFromPtr(self), ref_count, ref_count - 1 });
+            }
 
             self.ref_count = ref_count - 1;
 
@@ -71,6 +73,41 @@ pub fn NewRefCounted(comptime T: type, comptime deinit_fn: ?fn (self: *T) void,
 
             return ptr;
         }
+
+        pub fn refPtr(self: *T) Ptr {
+            self.ref();
+            return Ptr{ .__inner = self };
+        }
+
+        pub const Ptr = struct {
+            __inner: *T,
+
+            /// Initialize a new `T` on the heap. It will have a reference count of 1.
+            pub fn new(t: T) Ptr {
+                return Ptr{ .__inner = T.new(t) };
+            }
+
+            /// Borrow a reference to the allocation. Reference count is not
+            /// changed. Do not persist this pointer outside of the function
+            /// calling this method.
+            pub inline fn get(self: Ptr) *T {
+                return self.__inner;
+            }
+
+            /// Obtain a new reference to the allocation, increasing the
+            /// reference count in the process.
+            pub fn clone(self: Ptr) Ptr {
+                self.__inner.ref();
+                return self;
+            }
+
+            /// Release this reference to the shared allocation. If the
+            /// reference count goes below 1, the allocation is freed.
+            pub fn deinit(self: *Ptr) void {
+                self.__inner.deref();
+                self.* = undefined;
+            }
+        };
     };
 }
 

test/internal/ban-words.test.ts

@@ -28,7 +28,7 @@ const words: Record<string, { reason: string; limit?: number; regex?: boolean }>
   "== alloc.ptr": { reason: "The std.mem.Allocator context pointer can be undefined, which makes this comparison undefined behavior" },
   "!= alloc.ptr": { reason: "The std.mem.Allocator context pointer can be undefined, which makes this comparison undefined behavior" },
   [String.raw`: [a-zA-Z0-9_\.\*\?\[\]\(\)]+ = undefined,`]: { reason: "Do not default a struct field to undefined", limit: 245, regex: true },
-  "usingnamespace": { reason: "Zig deprecates this, and will not support it in incremental compilation.", limit: 494 },
+  "usingnamespace": { reason: "Zig deprecates this, and will not support it in incremental compilation.", limit: 495 },
 };
 const words_keys = [...Object.keys(words)];