From 2da57f6d7b30630d3441a07e1bc5c3445565de05 Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Wed, 24 Jul 2024 15:27:51 -0700 Subject: [PATCH] `napi_threadsafe_function` async tracker (#12780) --- .lldbinit | 2 + .vscode/settings.json | 5 ++- src/bun.js/ConsoleObject.zig | 3 +- src/bun.js/bindings/bindings.cpp | 1 + src/bun.js/event_loop.zig | 8 ++-- src/linker.lds | 1 + src/napi/napi.zig | 41 +++++++++++++------- src/symbols.dyn | 1 + src/symbols.txt | 1 + test/js/web/console/console-log.expected.txt | 1 + test/js/web/console/console-log.js | 3 ++ 11 files changed, 47 insertions(+), 20 deletions(-) create mode 100644 .lldbinit diff --git a/.lldbinit b/.lldbinit new file mode 100644 index 0000000000..2d527f4e63 --- /dev/null +++ b/.lldbinit @@ -0,0 +1,2 @@ +command script import src/deps/zig/tools/lldb_pretty_printers.py +command script import src/bun.js/WebKit/Tools/lldb/lldb_webkit.py diff --git a/.vscode/settings.json b/.vscode/settings.json index 91939e9e58..1701cb55df 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -42,8 +42,11 @@ "editor.defaultFormatter": "ziglang.vscode-zig", }, - // C++ + // lldb + "lldb.launch.initCommands": ["command source ${workspaceFolder}/.lldbinit"], "lldb.verboseLogging": false, + + // C++ "cmake.configureOnOpen": false, "C_Cpp.errorSquiggles": "enabled", "[cpp]": { diff --git a/src/bun.js/ConsoleObject.zig b/src/bun.js/ConsoleObject.zig index 04553fa537..4c0c584847 100644 --- a/src/bun.js/ConsoleObject.zig +++ b/src/bun.js/ConsoleObject.zig @@ -1283,6 +1283,7 @@ pub const Formatter = struct { writer.writeAll(end); // then skip the second % so we dont hit it again slice = slice[@min(slice.len, i + 1)..]; + len = @truncate(slice.len); i = 0; continue; }, @@ -1295,7 +1296,7 @@ pub const Formatter = struct { slice = slice[@min(slice.len, i + 1)..]; i = 0; hit_percent = true; - len = @as(u32, @truncate(slice.len)); + len = @truncate(slice.len); const next_value = this.remaining_values[0]; this.remaining_values = this.remaining_values[1..]; diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp index 5a1462e869..876d97b8fd 100644 --- a/src/bun.js/bindings/bindings.cpp +++ b/src/bun.js/bindings/bindings.cpp @@ -3056,6 +3056,7 @@ void JSC__JSPromise__reject(JSC__JSPromise* arg0, JSC__JSGlobalObject* globalObj JSC__JSValue JSValue2) { JSValue value = JSC::JSValue::decode(JSValue2); + ASSERT_WITH_MESSAGE(!value.isEmpty(), "Promise.reject cannot be called with a empty JSValue"); auto& vm = globalObject->vm(); ASSERT_WITH_MESSAGE(arg0->inherits(), "Argument is not a promise"); ASSERT_WITH_MESSAGE(arg0->status(vm) == JSC::JSPromise::Status::Pending, "Promise is already resolved or rejected"); diff --git a/src/bun.js/event_loop.zig b/src/bun.js/event_loop.zig index f8ec533624..468ae8ba67 100644 --- a/src/bun.js/event_loop.zig +++ b/src/bun.js/event_loop.zig @@ -771,7 +771,7 @@ pub const EventLoop = struct { waker: ?Waker = null, forever_timer: ?*uws.Timer = null, deferred_tasks: DeferredTaskQueue = .{}, - uws_loop: if (Environment.isWindows) *uws.Loop else void = undefined, + uws_loop: if (Environment.isWindows) ?*uws.Loop else void = if (Environment.isWindows) null else {}, debug: Debug = .{}, entered_event_loop_count: isize = 0, @@ -1324,7 +1324,7 @@ pub const EventLoop = struct { inline fn usocketsLoop(this: *const EventLoop) *uws.Loop { if (comptime Environment.isWindows) { - return this.uws_loop; + return this.uws_loop.?; } return this.virtual_machine.event_loop_handle.?; @@ -1572,7 +1572,9 @@ pub const EventLoop = struct { pub fn wakeup(this: *EventLoop) void { if (comptime Environment.isWindows) { - this.uws_loop.wakeup(); + if (this.uws_loop) |loop| { + loop.wakeup(); + } return; } diff --git a/src/linker.lds b/src/linker.lds index 27c44da312..9aee7b8227 100644 --- a/src/linker.lds +++ b/src/linker.lds @@ -5,6 +5,7 @@ BUN_1.1 { extern "C++" { v8::*; node::*; + JSC::CallFrame::describeFrame; }; local: *; diff --git a/src/napi/napi.zig b/src/napi/napi.zig index 25ace47662..ac89ea3349 100644 --- a/src/napi/napi.zig +++ b/src/napi/napi.zig @@ -972,7 +972,7 @@ pub export fn napi_resolve_deferred(env: napi_env, deferred: napi_deferred, reso log("napi_resolve_deferred", .{}); var prom = deferred.get(); prom.resolve(env, resolution); - deferred.*.strong.deinit(); + deferred.deinit(); bun.default_allocator.destroy(deferred); return .ok; } @@ -980,7 +980,7 @@ pub export fn napi_reject_deferred(env: napi_env, deferred: napi_deferred, rejec log("napi_reject_deferred", .{}); var prom = deferred.get(); prom.reject(env, rejection); - deferred.*.strong.deinit(); + deferred.deinit(); bun.default_allocator.destroy(deferred); return .ok; } @@ -990,9 +990,8 @@ pub export fn napi_is_promise(_: napi_env, value: napi_value, is_promise_: ?*boo return invalidArg(); }; - if (value.isEmptyOrUndefinedOrNull()) { - is_promise.* = false; - return .ok; + if (value.isEmpty()) { + return invalidArg(); } is_promise.* = value.asAnyPromise() != null; @@ -1422,10 +1421,10 @@ pub const ThreadSafeFunction = struct { thread_count: usize = 0, owning_thread_lock: Lock = Lock.init(), event_loop: *JSC.EventLoop, + tracker: JSC.AsyncTaskTracker, env: napi_env, - finalizer_task: JSC.AnyTask = undefined, finalizer: Finalizer = Finalizer{ .fun = null, .data = null }, channel: Queue, @@ -1503,18 +1502,30 @@ pub const ThreadSafeFunction = struct { pub fn call(this: *ThreadSafeFunction) void { const task = this.channel.tryReadItem() catch null orelse return; + const vm = this.event_loop.virtual_machine; + const globalObject = this.env; + + this.tracker.willDispatch(globalObject); + defer this.tracker.didDispatch(globalObject); + switch (this.callback) { .js => |js_function| { if (js_function.isEmptyOrUndefinedOrNull()) { return; } - const err = js_function.call(this.env, &.{}); + const err = js_function.call(globalObject, &.{}); if (err.isAnyError()) { - _ = this.env.bunVM().uncaughtException(this.env, err, false); + _ = vm.uncaughtException(globalObject, err, false); } }, .c => |cb| { - cb.napi_threadsafe_function_call_js(this.env, cb.js, this.ctx, task); + if (comptime bun.Environment.isDebug) { + const str = cb.js.toBunString(globalObject); + defer str.deref(); + log("call() {}", .{str}); + } + + cb.napi_threadsafe_function_call_js(globalObject, cb.js, this.ctx, task); }, } } @@ -1584,7 +1595,6 @@ pub const ThreadSafeFunction = struct { } if (mode == .abort or this.thread_count == 0) { - this.finalizer_task = JSC.AnyTask{ .ctx = this, .callback = finalize }; this.event_loop.enqueueTaskConcurrent(JSC.ConcurrentTask.fromCallback(this, finalize)); } @@ -1618,27 +1628,30 @@ pub export fn napi_create_threadsafe_function( func.protect(); } + const vm = env.bunVM(); var function = bun.default_allocator.create(ThreadSafeFunction) catch return genericFailure(); function.* = .{ - .event_loop = env.bunVM().eventLoop(), + .event_loop = vm.eventLoop(), .env = env, .callback = if (call_js_cb) |c| .{ .c = .{ .napi_threadsafe_function_call_js = c, - .js = if (func == .zero) JSC.JSValue.jsUndefined() else func, + .js = if (func == .zero) JSC.JSValue.jsUndefined() else func.withAsyncContextIfNeeded(env), }, } else .{ - .js = if (func == .zero) JSC.JSValue.jsUndefined() else func, + .js = if (func == .zero) JSC.JSValue.jsUndefined() else func.withAsyncContextIfNeeded(env), }, .ctx = context, .channel = ThreadSafeFunction.Queue.init(max_queue_size, bun.default_allocator), .thread_count = initial_thread_count, .poll_ref = Async.KeepAlive.init(), + .tracker = JSC.AsyncTaskTracker.init(vm), }; function.finalizer = .{ .data = thread_finalize_data, .fun = thread_finalize_cb }; // nodejs by default keeps the event loop alive until the thread-safe function is unref'd function.ref(); + function.tracker.didSchedule(vm.global); result.* = function; return .ok; @@ -1673,14 +1686,12 @@ pub export fn napi_release_threadsafe_function(func: napi_threadsafe_function, m pub export fn napi_unref_threadsafe_function(env: napi_env, func: napi_threadsafe_function) napi_status { log("napi_unref_threadsafe_function", .{}); bun.assert(func.event_loop.global == env); - func.unref(); return .ok; } pub export fn napi_ref_threadsafe_function(env: napi_env, func: napi_threadsafe_function) napi_status { log("napi_ref_threadsafe_function", .{}); bun.assert(func.event_loop.global == env); - func.ref(); return .ok; } diff --git a/src/symbols.dyn b/src/symbols.dyn index 44160f2867..2d16df145d 100644 --- a/src/symbols.dyn +++ b/src/symbols.dyn @@ -154,4 +154,5 @@ __ZN2v87Isolate17GetCurrentContextEv; __ZN4node25AddEnvironmentCleanupHookEPN2v87IsolateEPFvPvES3_; __ZN4node28RemoveEnvironmentCleanupHookEPN2v87IsolateEPFvPvES3_; + __ZN3JSC9CallFrame13describeFrameEv; }; \ No newline at end of file diff --git a/src/symbols.txt b/src/symbols.txt index 577035fa93..2e24afa1c4 100644 --- a/src/symbols.txt +++ b/src/symbols.txt @@ -153,3 +153,4 @@ __ZN2v87Isolate13TryGetCurrentEv __ZN2v87Isolate17GetCurrentContextEv __ZN4node25AddEnvironmentCleanupHookEPN2v87IsolateEPFvPvES3_ __ZN4node28RemoveEnvironmentCleanupHookEPN2v87IsolateEPFvPvES3_ +__ZN3JSC9CallFrame13describeFrameEv \ No newline at end of file diff --git a/test/js/web/console/console-log.expected.txt b/test/js/web/console/console-log.expected.txt index 167512e7bd..86cff95206 100644 --- a/test/js/web/console/console-log.expected.txt +++ b/test/js/web/console/console-log.expected.txt @@ -284,3 +284,4 @@ Hello NaN % 1 Hello NaN %j 1 Hello \5 6, Hello %i 5 6 +%d 1 diff --git a/test/js/web/console/console-log.js b/test/js/web/console/console-log.js index 46357f219d..32a817bc78 100644 --- a/test/js/web/console/console-log.js +++ b/test/js/web/console/console-log.js @@ -261,3 +261,6 @@ console.log("Hello %i %", [1, 2, 3, 4], 1); console.log("Hello %i %j", [1, 2, 3, 4], 1); console.log("Hello \\%i %i,", 5, 6); console.log("Hello %%i %i", 5, 6); + +// doesn't go out of bounds when printing +console.log("%%d", 1);