From 1b468a63eb3ed16e5580618fabbeda66a4add23c Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Mon, 28 Oct 2024 19:10:00 -0700 Subject: [PATCH 1/6] Copy subset of changes from node http branch --- .../bun-usockets/src/eventing/epoll_kqueue.c | 11 ++++ .../bun-usockets/src/internal/loop_data.h | 1 + packages/bun-usockets/src/loop.c | 1 + src/bun.js/bindings/BunJSCEventLoop.cpp | 18 ++++++ .../bindings/JSCUSocketsLoopIntegration.cpp | 30 ---------- src/bun.js/bindings/bindings.zig | 16 ++++++ src/bun.js/event_loop.zig | 1 + src/bun.js/module_loader.zig | 6 ++ src/bun.js/node/node_zlib_binding.zig | 55 +++++++++++++------ src/bun.zig | 4 +- src/deps/uws.zig | 1 + .../next-pages/test/dev-server.test.ts | 2 +- 12 files changed, 95 insertions(+), 51 deletions(-) create mode 100644 src/bun.js/bindings/BunJSCEventLoop.cpp delete mode 100644 src/bun.js/bindings/JSCUSocketsLoopIntegration.cpp diff --git a/packages/bun-usockets/src/eventing/epoll_kqueue.c b/packages/bun-usockets/src/eventing/epoll_kqueue.c index 9395e190ea0a29..9091cef8b7e7b1 100644 --- a/packages/bun-usockets/src/eventing/epoll_kqueue.c +++ b/packages/bun-usockets/src/eventing/epoll_kqueue.c @@ -230,6 +230,8 @@ void us_loop_run(struct us_loop_t *loop) { } } +extern int Bun__JSC_onBeforeWait(void*); +extern void Bun__JSC_onAfterWait(void*); void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout) { if (loop->num_polls == 0) @@ -246,6 +248,11 @@ void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout /* Emit pre callback */ us_internal_loop_pre(loop); + int needs_after_wait = 0; + if (loop->data.jsc_vm) { + needs_after_wait = Bun__JSC_onBeforeWait(loop->data.jsc_vm); + } + /* Fetch ready polls */ #ifdef LIBUS_USE_EPOLL @@ -256,6 +263,10 @@ void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout } while (IS_EINTR(loop->num_ready_polls)); #endif + if (needs_after_wait) { + Bun__JSC_onAfterWait(loop->data.jsc_vm); + } + /* Iterate ready polls, dispatching them by type */ for (loop->current_ready_poll = 0; loop->current_ready_poll < loop->num_ready_polls; loop->current_ready_poll++) { struct us_poll_t *poll = GET_READY_POLL(loop, loop->current_ready_poll); diff --git a/packages/bun-usockets/src/internal/loop_data.h b/packages/bun-usockets/src/internal/loop_data.h index 1f0a3adb767907..ddbaee9ffc76b2 100644 --- a/packages/bun-usockets/src/internal/loop_data.h +++ b/packages/bun-usockets/src/internal/loop_data.h @@ -44,6 +44,7 @@ struct us_internal_loop_data_t { char parent_tag; /* We do not care if this flips or not, it doesn't matter */ size_t iteration_nr; + void* jsc_vm; }; #endif // LOOP_DATA_H diff --git a/packages/bun-usockets/src/loop.c b/packages/bun-usockets/src/loop.c index 9bd315822b7b0e..e4b7845f239e9a 100644 --- a/packages/bun-usockets/src/loop.c +++ b/packages/bun-usockets/src/loop.c @@ -48,6 +48,7 @@ void us_internal_loop_data_init(struct us_loop_t *loop, void (*wakeup_cb)(struct loop->data.parent_tag = 0; loop->data.closed_context_head = 0; + loop->data.jsc_vm = 0; loop->data.wakeup_async = us_internal_create_async(loop, 1, 0); us_internal_async_set(loop->data.wakeup_async, (void (*)(struct us_internal_async *)) wakeup_cb); diff --git a/src/bun.js/bindings/BunJSCEventLoop.cpp b/src/bun.js/bindings/BunJSCEventLoop.cpp new file mode 100644 index 00000000000000..7a3cf95b2a01ee --- /dev/null +++ b/src/bun.js/bindings/BunJSCEventLoop.cpp @@ -0,0 +1,18 @@ +#include "root.h" + +#include +#include + +extern "C" int Bun__JSC_onBeforeWait(JSC::VM* vm) +{ + if (vm->heap.hasAccess()) { + vm->heap.releaseAccess(); + return 1; + } + return 0; +} + +extern "C" void Bun__JSC_onAfterWait(JSC::VM* vm) +{ + vm->heap.acquireAccess(); +} diff --git a/src/bun.js/bindings/JSCUSocketsLoopIntegration.cpp b/src/bun.js/bindings/JSCUSocketsLoopIntegration.cpp deleted file mode 100644 index 9566a289099c8c..00000000000000 --- a/src/bun.js/bindings/JSCUSocketsLoopIntegration.cpp +++ /dev/null @@ -1,30 +0,0 @@ -#include "root.h" -#include "JavaScriptCore/VM.h" - -// On Linux, signals are used to suspend/resume threads in JavaScriptCore -// When `.acquireAccess` is called, the signal might be raised. -// This causes issues with LLDB which might catch the signal. -// So we want to avoid that, we really only want this code to be executed when the debugger is attached -// But it's pretty hard to tell if LLDB is attached or not, so we just disable this code on Linux when in debug mode -#ifndef ACQUIRE_RELEASE_HEAP_ACCESS -#if OS(DARWIN) -#define ACQUIRE_RELEASE_HEAP_ACCESS 1 -#else -#ifndef BUN_DEBUG -#define ACQUIRE_RELEASE_HEAP_ACCESS 1 -#endif -#endif -#endif - -extern "C" void bun_on_tick_before(JSC::VM* vm) -{ -#if ACQUIRE_RELEASE_HEAP_ACCESS - // vm->heap.releaseAccess(); -#endif -} -extern "C" void bun_on_tick_after(JSC::VM* vm) -{ -#if ACQUIRE_RELEASE_HEAP_ACCESS - // vm->heap.acquireAccess(); -#endif -} diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig index 37a24a72bec108..d3ad3bfb0b7687 100644 --- a/src/bun.js/bindings/bindings.zig +++ b/src/bun.js/bindings/bindings.zig @@ -6161,6 +6161,22 @@ pub const VM = extern struct { LargeHeap = 1, }; + extern fn Bun__JSC_onBeforeWait(vm: *VM) i32; + extern fn Bun__JSC_onAfterWait(vm: *VM) void; + pub const ReleaseHeapAccess = struct { + vm: *VM, + needs_to_release: bool, + pub fn acquire(this: *const ReleaseHeapAccess) void { + if (this.needs_to_release) { + Bun__JSC_onAfterWait(this.vm); + } + } + }; + + pub fn releaseHeapAccess(vm: *VM) ReleaseHeapAccess { + return .{ .vm = vm, .needs_to_release = Bun__JSC_onBeforeWait(vm) != 0 }; + } + pub fn create(heap_type: HeapType) *VM { return cppFn("create", .{@intFromEnum(heap_type)}); } diff --git a/src/bun.js/event_loop.zig b/src/bun.js/event_loop.zig index bca57c7f3e9e94..cd3d8a39aec256 100644 --- a/src/bun.js/event_loop.zig +++ b/src/bun.js/event_loop.zig @@ -545,6 +545,7 @@ pub const GarbageCollectionController = struct { const actual = uws.Loop.get(); this.gc_timer = uws.Timer.createFallthrough(actual, this); this.gc_repeating_timer = uws.Timer.createFallthrough(actual, this); + actual.internal_loop_data.jsc_vm = vm.jsc; if (comptime Environment.isDebug) { if (bun.getenvZ("BUN_TRACK_LAST_FN_NAME") != null) { diff --git a/src/bun.js/module_loader.zig b/src/bun.js/module_loader.zig index f35098f03117fc..2f378ec45a7ae7 100644 --- a/src/bun.js/module_loader.zig +++ b/src/bun.js/module_loader.zig @@ -1646,6 +1646,12 @@ pub const ModuleLoader = struct { var parse_result: ParseResult = switch (disable_transpilying or (loader == .json and !path.isJSONCFile())) { inline else => |return_file_only| brk: { + const heap_access = if (!disable_transpilying) + jsc_vm.jsc.releaseHeapAccess() + else + JSC.VM.ReleaseHeapAccess{ .vm = jsc_vm.jsc, .needs_to_release = false }; + defer heap_access.acquire(); + break :brk jsc_vm.bundler.parseMaybeReturnFileOnly( parse_options, null, diff --git a/src/bun.js/node/node_zlib_binding.zig b/src/bun.js/node/node_zlib_binding.zig index 5337a6427146bf..02aca9890f107c 100644 --- a/src/bun.js/node/node_zlib_binding.zig +++ b/src/bun.js/node/node_zlib_binding.zig @@ -114,35 +114,25 @@ pub fn CompressionStream(comptime T: type) type { // const vm = globalThis.bunVM(); - var task = AsyncJob.new(.{ - .binding = this, - }); + this.task = .{ .callback = &AsyncJob.runTask }; this.poll_ref.ref(vm); - JSC.WorkPool.schedule(&task.task); + JSC.WorkPool.schedule(&this.task); return .undefined; } const AsyncJob = struct { - task: JSC.WorkPoolTask = .{ .callback = &runTask }, - binding: *T, - - pub usingnamespace bun.New(@This()); - - pub fn runTask(this: *JSC.WorkPoolTask) void { - var job: *AsyncJob = @fieldParentPtr("task", this); - job.run(); - job.destroy(); + pub fn runTask(task: *JSC.WorkPoolTask) void { + const this: *T = @fieldParentPtr("task", task); + AsyncJob.run(this); } - pub fn run(job: *AsyncJob) void { - const this = job.binding; + pub fn run(this: *T) void { const globalThis: *JSC.JSGlobalObject = this.globalThis; const vm = globalThis.bunVMConcurrently(); this.stream.doWork(); - this.poll_ref.refConcurrently(vm); vm.enqueueTaskConcurrent(JSC.ConcurrentTask.create(JSC.Task.init(this))); } }; @@ -294,6 +284,29 @@ pub fn CompressionStream(comptime T: type) type { pub const NativeZlib = JSC.Codegen.JSNativeZlib.getConstructor; +const CountedKeepAlive = struct { + keep_alive: bun.Async.KeepAlive = .{}, + ref_count: u32 = 0, + + pub fn ref(this: *@This(), vm: *JSC.VirtualMachine) void { + if (this.ref_count == 0) { + this.keep_alive.ref(vm); + } + this.ref_count += 1; + } + + pub fn unref(this: *@This(), vm: *JSC.VirtualMachine) void { + this.ref_count -= 1; + if (this.ref_count == 0) { + this.keep_alive.unref(vm); + } + } + + pub fn deinit(this: *@This()) void { + this.keep_alive.disable(); + } +}; + pub const SNativeZlib = struct { pub usingnamespace bun.NewRefCounted(@This(), deinit); pub usingnamespace JSC.Codegen.JSNativeZlib; @@ -306,11 +319,12 @@ pub const SNativeZlib = struct { write_result: ?[*]u32 = null, write_callback: JSC.Strong = .{}, onerror_value: JSC.Strong = .{}, - poll_ref: bun.Async.KeepAlive = .{}, + poll_ref: CountedKeepAlive = .{}, this_value: JSC.Strong = .{}, write_in_progress: bool = false, pending_close: bool = false, closed: bool = false, + task: JSC.WorkPoolTask = .{ .callback = undefined }, pub fn constructor(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) ?*@This() { const arguments = callframe.argumentsUndef(4).ptr; @@ -392,6 +406,7 @@ pub const SNativeZlib = struct { pub fn deinit(this: *@This()) void { this.write_callback.deinit(); this.onerror_value.deinit(); + this.poll_ref.deinit(); this.destroy(); } }; @@ -662,11 +677,14 @@ pub const SNativeBrotli = struct { write_result: ?[*]u32 = null, write_callback: JSC.Strong = .{}, onerror_value: JSC.Strong = .{}, - poll_ref: bun.Async.KeepAlive = .{}, + poll_ref: CountedKeepAlive = .{}, this_value: JSC.Strong = .{}, write_in_progress: bool = false, pending_close: bool = false, closed: bool = false, + task: JSC.WorkPoolTask = .{ + .callback = undefined, + }, pub fn constructor(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) ?*@This() { const arguments = callframe.argumentsUndef(1).ptr; @@ -752,6 +770,7 @@ pub const SNativeBrotli = struct { pub fn deinit(this: *@This()) void { this.write_callback.deinit(); this.onerror_value.deinit(); + this.poll_ref.deinit(); this.destroy(); } }; diff --git a/src/bun.zig b/src/bun.zig index b21c856020954f..f7d9cda39630db 100644 --- a/src/bun.zig +++ b/src/bun.zig @@ -3146,8 +3146,8 @@ pub fn NewRefCounted(comptime T: type, comptime deinit_fn: ?fn (self: *T) void) const ptr = bun.new(T, t); if (Environment.enable_logs) { - if (ptr.ref_count != 1) { - Output.panic("Expected ref_count to be 1, got {d}", .{ptr.ref_count}); + if (ptr.ref_count == 0) { + Output.panic("Expected ref_count to be > 0, got {d}", .{ptr.ref_count}); } } diff --git a/src/deps/uws.zig b/src/deps/uws.zig index f44f02f5c7f4ab..e6d75a32443a12 100644 --- a/src/deps/uws.zig +++ b/src/deps/uws.zig @@ -55,6 +55,7 @@ pub const InternalLoopData = extern struct { parent_ptr: ?*anyopaque, parent_tag: c_char, iteration_nr: usize, + jsc_vm: ?*JSC.VM, pub fn recvSlice(this: *InternalLoopData) []u8 { return this.recv_buf[0..LIBUS_RECV_BUFFER_LENGTH]; diff --git a/test/integration/next-pages/test/dev-server.test.ts b/test/integration/next-pages/test/dev-server.test.ts index ae7fadb38dcb69..6a3c553c6e15ed 100644 --- a/test/integration/next-pages/test/dev-server.test.ts +++ b/test/integration/next-pages/test/dev-server.test.ts @@ -79,7 +79,7 @@ async function getDevServerURL() { readStream() .catch(e => reject(e)) .finally(() => { - dev_server.unref?.(); + dev_server?.unref?.(); }); await promise; return baseUrl; From 9f4bcaf5d81b4d84f89cee147dcd1454cf12accd Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Mon, 28 Oct 2024 20:18:29 -0700 Subject: [PATCH 2/6] try this --- src/async/posix_event_loop.zig | 4 ++-- src/bun.js/web_worker.zig | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/async/posix_event_loop.zig b/src/async/posix_event_loop.zig index 491e09ce7b2fcc..bb4c4286f121b6 100644 --- a/src/async/posix_event_loop.zig +++ b/src/async/posix_event_loop.zig @@ -55,11 +55,11 @@ pub const KeepAlive = struct { this.status = .inactive; if (comptime @TypeOf(event_loop_ctx_) == JSC.EventLoopHandle) { - event_loop_ctx_.loop().subActive(1); + event_loop_ctx_.loop().unref(); return; } const event_loop_ctx = JSC.AbstractVM(event_loop_ctx_); - event_loop_ctx.platformEventLoop().subActive(1); + event_loop_ctx.platformEventLoop().unref(); } /// From another thread, Prevent a poll from keeping the process alive. diff --git a/src/bun.js/web_worker.zig b/src/bun.js/web_worker.zig index 848b84dfde7e54..9be327684a114e 100644 --- a/src/bun.js/web_worker.zig +++ b/src/bun.js/web_worker.zig @@ -459,7 +459,9 @@ pub const WebWorker = struct { var exit_code: i32 = 0; var globalObject: ?*JSC.JSGlobalObject = null; var vm_to_deinit: ?*JSC.VirtualMachine = null; + var loop: ?*bun.uws.Loop = null; if (this.vm) |vm| { + loop = vm.uwsLoop(); this.vm = null; vm.is_shutting_down = true; vm.onExit(); @@ -470,6 +472,9 @@ pub const WebWorker = struct { var arena = this.arena; WebWorker__dispatchExit(globalObject, cpp_worker, exit_code); + if (loop) |loop_| { + loop_.internal_loop_data.jsc_vm = null; + } bun.uws.onThreadExit(); this.deinit(); From ae7feeda9541e4ce654cdb4be3857b814a20fbdb Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Tue, 29 Oct 2024 12:03:54 -0700 Subject: [PATCH 3/6] Update expectBundled.ts --- test/bundler/expectBundled.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/bundler/expectBundled.ts b/test/bundler/expectBundled.ts index 5f0052fb452838..9a49939e09f76a 100644 --- a/test/bundler/expectBundled.ts +++ b/test/bundler/expectBundled.ts @@ -556,6 +556,12 @@ function expectBundled( let root = path.join( tempDirectory, id + .replaceAll("\\", "/") + .replaceAll(":", "-") + .replaceAll(" ", "-") + .replaceAll("\r\n", "-") + .replaceAll("\n", "-") + .replaceAll(".", "-") .split("/") .map(a => filenamify(a)) .join("/"), From 830a62f81007cb9286f5286bdd68de3084be2844 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Tue, 29 Oct 2024 12:14:29 -0700 Subject: [PATCH 4/6] Update doesnt_crash.test.ts --- test/js/bun/css/doesnt_crash.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/js/bun/css/doesnt_crash.test.ts b/test/js/bun/css/doesnt_crash.test.ts index 2293e87e60ea9e..b811fd8d93c968 100644 --- a/test/js/bun/css/doesnt_crash.test.ts +++ b/test/js/bun/css/doesnt_crash.test.ts @@ -16,10 +16,11 @@ describe("doesnt_crash", async () => { console.log("Tempdir", temp_dir); files.map(file => { - const outfile1 = path.join(temp_dir, file).replaceAll("\\", "/"); - const outfile2 = path.join(temp_dir, "lmao1-" + file).replaceAll("\\", "/"); - const outfile3 = path.join(temp_dir, "lmao2-" + file).replaceAll("\\", "/"); - const outfile4 = path.join(temp_dir, "lmao3-" + file).replaceAll("\\", "/"); + const outfile1 = path.join(temp_dir, "file-1" + path.basename(file)).replaceAll("\\", "/"); + const outfile2 = path.join(temp_dir, "file-2" + path.basename(file)).replaceAll("\\", "/"); + const outfile3 = path.join(temp_dir, "file-3" + path.basename(file)).replaceAll("\\", "/"); + const outfile4 = path.join(temp_dir, "file-4" + path.basename(file)).replaceAll("\\", "/"); + test(file, async () => { { const { stdout, stderr, exitCode } = From 53839715774178384c84ac780e66a9f815923276 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Tue, 29 Oct 2024 12:21:57 -0700 Subject: [PATCH 5/6] Update doesnt_crash.test.ts --- test/js/bun/css/doesnt_crash.test.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/test/js/bun/css/doesnt_crash.test.ts b/test/js/bun/css/doesnt_crash.test.ts index b811fd8d93c968..6b2da1e315039f 100644 --- a/test/js/bun/css/doesnt_crash.test.ts +++ b/test/js/bun/css/doesnt_crash.test.ts @@ -15,16 +15,17 @@ describe("doesnt_crash", async () => { files = readdirSync(files_dir).map(file => path.join(files_dir, file)); console.log("Tempdir", temp_dir); - files.map(file => { - const outfile1 = path.join(temp_dir, "file-1" + path.basename(file)).replaceAll("\\", "/"); - const outfile2 = path.join(temp_dir, "file-2" + path.basename(file)).replaceAll("\\", "/"); - const outfile3 = path.join(temp_dir, "file-3" + path.basename(file)).replaceAll("\\", "/"); - const outfile4 = path.join(temp_dir, "file-4" + path.basename(file)).replaceAll("\\", "/"); + files.map(absolute => { + const file = path.basename(absolute); + const outfile1 = path.join(temp_dir, "file-1" + file).replaceAll("\\", "/"); + const outfile2 = path.join(temp_dir, "file-2" + file).replaceAll("\\", "/"); + const outfile3 = path.join(temp_dir, "file-3" + file).replaceAll("\\", "/"); + const outfile4 = path.join(temp_dir, "file-4" + file).replaceAll("\\", "/"); test(file, async () => { { const { stdout, stderr, exitCode } = - await Bun.$`${bunExe()} build --experimental-css ${file} --outfile=${outfile1}`.quiet().env(bunEnv); + await Bun.$`${bunExe()} build --experimental-css ${absolute} --outfile=${outfile1}`.quiet().env(bunEnv); expect(exitCode).toBe(0); expect(stdout.toString()).not.toContain("error"); expect(stderr.toString()).toBeEmpty(); @@ -40,7 +41,9 @@ describe("doesnt_crash", async () => { test(`(minify) ${file}`, async () => { { const { stdout, stderr, exitCode } = - await Bun.$`${bunExe()} build --experimental-css ${file} --minify --outfile=${outfile3}`.quiet().env(bunEnv); + await Bun.$`${bunExe()} build --experimental-css ${absolute} --minify --outfile=${outfile3}` + .quiet() + .env(bunEnv); expect(exitCode).toBe(0); expect(stdout.toString()).not.toContain("error"); expect(stderr.toString()).toBeEmpty(); From bcab095688e77707312af2e2d5719f7cf64fde57 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Tue, 29 Oct 2024 12:28:02 -0700 Subject: [PATCH 6/6] Update doesnt_crash.test.ts --- test/js/bun/css/doesnt_crash.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/js/bun/css/doesnt_crash.test.ts b/test/js/bun/css/doesnt_crash.test.ts index 6b2da1e315039f..c418f74e22f1fb 100644 --- a/test/js/bun/css/doesnt_crash.test.ts +++ b/test/js/bun/css/doesnt_crash.test.ts @@ -16,6 +16,7 @@ describe("doesnt_crash", async () => { console.log("Tempdir", temp_dir); files.map(absolute => { + absolute = absolute.replaceAll("\\", "/"); const file = path.basename(absolute); const outfile1 = path.join(temp_dir, "file-1" + file).replaceAll("\\", "/"); const outfile2 = path.join(temp_dir, "file-2" + file).replaceAll("\\", "/");