From 67b64c3334ac32522f20767c2f8b846e194ea26f Mon Sep 17 00:00:00 2001 From: Braden Everson Date: Mon, 19 May 2025 18:44:57 -0500 Subject: [PATCH 1/6] Update TextDecoder's constructor to Handle Undefined (#19708) Co-authored-by: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> --- src/bun.js/webcore/TextDecoder.zig | 58 ++++++++++------------- test/internal/ban-words.test.ts | 2 +- test/js/web/encoding/text-decoder.test.js | 6 +++ 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/bun.js/webcore/TextDecoder.zig b/src/bun.js/webcore/TextDecoder.zig index 873800b5c3..ebaee756dd 100644 --- a/src/bun.js/webcore/TextDecoder.zig +++ b/src/bun.js/webcore/TextDecoder.zig @@ -280,46 +280,40 @@ fn decodeSlice(this: *TextDecoder, globalThis: *JSC.JSGlobalObject, buffer_slice } pub fn constructor(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!*TextDecoder { - var args_ = callframe.arguments_old(2); - var arguments: []const JSC.JSValue = args_.ptr[0..args_.len]; + const encoding_value, const options_value = callframe.argumentsAsArray(2); var decoder = TextDecoder{}; - if (arguments.len > 0) { - // encoding - if (arguments[0].isString()) { - var str = try arguments[0].toSlice(globalThis, bun.default_allocator); - defer if (str.isAllocated()) str.deinit(); + if (encoding_value.isString()) { + var str = try encoding_value.toSlice(globalThis, bun.default_allocator); + defer str.deinit(); - if (EncodingLabel.which(str.slice())) |label| { - decoder.encoding = label; - } else { - return globalThis.ERR(.ENCODING_NOT_SUPPORTED, "Unsupported encoding label \"{s}\"", .{str.slice()}).throw(); - } - } else if (arguments[0].isUndefined()) { - // default to utf-8 - decoder.encoding = EncodingLabel.@"UTF-8"; + if (EncodingLabel.which(str.slice())) |label| { + decoder.encoding = label; } else { - return globalThis.throwInvalidArguments("TextDecoder(encoding) label is invalid", .{}); + return globalThis.ERR(.ENCODING_NOT_SUPPORTED, "Unsupported encoding label \"{s}\"", .{str.slice()}).throw(); + } + } else if (encoding_value.isUndefined()) { + // default to utf-8 + decoder.encoding = EncodingLabel.@"UTF-8"; + } else { + return globalThis.throwInvalidArguments("TextDecoder(encoding) label is invalid", .{}); + } + + if (!options_value.isUndefined()) { + if (!options_value.isObject()) { + return globalThis.throwInvalidArguments("TextDecoder(options) is invalid", .{}); } - if (arguments.len >= 2) { - const options = arguments[1]; + if (try options_value.get(globalThis, "fatal")) |fatal| { + decoder.fatal = fatal.toBoolean(); + } - if (!options.isObject()) { - return globalThis.throwInvalidArguments("TextDecoder(options) is invalid", .{}); - } - - if (try options.get(globalThis, "fatal")) |fatal| { - decoder.fatal = fatal.toBoolean(); - } - - if (try options.get(globalThis, "ignoreBOM")) |ignoreBOM| { - if (ignoreBOM.isBoolean()) { - decoder.ignore_bom = ignoreBOM.asBoolean(); - } else { - return globalThis.throwInvalidArguments("TextDecoder(options) ignoreBOM is invalid. Expected boolean value", .{}); - } + if (try options_value.get(globalThis, "ignoreBOM")) |ignoreBOM| { + if (ignoreBOM.isBoolean()) { + decoder.ignore_bom = ignoreBOM.asBoolean(); + } else { + return globalThis.throwInvalidArguments("TextDecoder(options) ignoreBOM is invalid. Expected boolean value", .{}); } } } diff --git a/test/internal/ban-words.test.ts b/test/internal/ban-words.test.ts index 31d7e9a8e7..3ed36e67a9 100644 --- a/test/internal/ban-words.test.ts +++ b/test/internal/ban-words.test.ts @@ -41,7 +41,7 @@ const words: Record "std.fs.File": { reason: "Prefer bun.sys + bun.FD instead of std.fs", limit: 64 }, ".stdFile()": { reason: "Prefer bun.sys + bun.FD instead of std.fs.File. Zig hides 'errno' when Bun wants to match libuv", limit: 18 }, ".stdDir()": { reason: "Prefer bun.sys + bun.FD instead of std.fs.File. Zig hides 'errno' when Bun wants to match libuv", limit: 48 }, - ".arguments_old(": { reason: "Please migrate to .argumentsAsArray() or another argument API", limit: 286 }, + ".arguments_old(": { reason: "Please migrate to .argumentsAsArray() or another argument API", limit: 285 }, "// autofix": { reason: "Evaluate if this variable should be deleted entirely or explicitly discarded.", limit: 176 }, }; const words_keys = [...Object.keys(words)]; diff --git a/test/js/web/encoding/text-decoder.test.js b/test/js/web/encoding/text-decoder.test.js index ca27e07890..42738c4e88 100644 --- a/test/js/web/encoding/text-decoder.test.js +++ b/test/js/web/encoding/text-decoder.test.js @@ -298,6 +298,12 @@ describe("TextDecoder", () => { const decoder = new TextDecoder(undefined); expect(decoder.encoding).toBe("utf-8"); }); + + it("should support undefined options", () => { + expect(() => { + const decoder = new TextDecoder("utf-8", undefined); + }).not.toThrow(); + }); }); describe("TextDecoder ignoreBOM", () => { From 33be08bde89cff9b9283ef933f3913dcf1e5baba Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Mon, 19 May 2025 17:05:10 -0700 Subject: [PATCH 2/6] Fix RuntimeError.from return value (#19777) Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: dylan-conway --- packages/bun-error/runtime-error.ts | 4 ++-- test/js/bun/runtime-error.test.ts | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 test/js/bun/runtime-error.test.ts diff --git a/packages/bun-error/runtime-error.ts b/packages/bun-error/runtime-error.ts index 4a739edf3e..81041129d8 100644 --- a/packages/bun-error/runtime-error.ts +++ b/packages/bun-error/runtime-error.ts @@ -42,11 +42,11 @@ export default class RuntimeError { original: Error; stack: StackFrame[]; - static from(error: Error) { + static from(error: Error): RuntimeError { const runtime = new RuntimeError(); runtime.original = error; runtime.stack = this.parseStack(error); - return RuntimeError; + return runtime; } /** diff --git a/test/js/bun/runtime-error.test.ts b/test/js/bun/runtime-error.test.ts new file mode 100644 index 0000000000..36c01859de --- /dev/null +++ b/test/js/bun/runtime-error.test.ts @@ -0,0 +1,9 @@ +import { expect, test } from "bun:test"; +import RuntimeError from "../../../packages/bun-error/runtime-error"; + +test("RuntimeError.from returns instance", () => { + const err = new Error("boom"); + const runtime = RuntimeError.from(err); + expect(runtime.original).toBe(err); + expect(Array.isArray(runtime.stack)).toBe(true); +}); From 21f238a82728c71253aa224b4035882bd5b26ad6 Mon Sep 17 00:00:00 2001 From: Ashcon Partovi Date: Tue, 20 May 2025 10:53:57 -0700 Subject: [PATCH 3/6] cmake: Move sources to their own folder (#19776) --- .github/CODEOWNERS | 18 ++++++++--------- cmake/{ => sources}/BakeRuntimeSources.txt | 0 cmake/{ => sources}/BindgenSources.txt | 0 cmake/{ => sources}/BunErrorSources.txt | 0 cmake/{ => sources}/CSources.txt | 3 +-- cmake/{ => sources}/CxxSources.txt | 0 .../JavaScriptCodegenSources.txt | 0 cmake/{ => sources}/JavaScriptSources.txt | 0 cmake/{ => sources}/NodeFallbacksSources.txt | 0 .../ZigGeneratedClassesSources.txt | 0 cmake/{ => sources}/ZigSources.txt | 1 + cmake/targets/BuildBun.cmake | 20 +++++++++---------- scripts/glob-sources.mjs | 2 +- 13 files changed, 22 insertions(+), 22 deletions(-) rename cmake/{ => sources}/BakeRuntimeSources.txt (100%) rename cmake/{ => sources}/BindgenSources.txt (100%) rename cmake/{ => sources}/BunErrorSources.txt (100%) rename cmake/{ => sources}/CSources.txt (87%) rename cmake/{ => sources}/CxxSources.txt (100%) rename cmake/{ => sources}/JavaScriptCodegenSources.txt (100%) rename cmake/{ => sources}/JavaScriptSources.txt (100%) rename cmake/{ => sources}/NodeFallbacksSources.txt (100%) rename cmake/{ => sources}/ZigGeneratedClassesSources.txt (100%) rename cmake/{ => sources}/ZigSources.txt (99%) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index d4b774ba36..e7850387a0 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,18 +1,18 @@ # Project -.github/CODEOWNERS @Jarred-Sumner +/.github/CODEOWNERS @Jarred-Sumner # Build system -CMakeLists.txt @Electroid -cmake/ @Electroid -scripts/ @Electroid +/CMakeLists.txt @Electroid +/cmake/*.cmake @Electroid +/scripts/ @Electroid # CI -.buildkite/ @Electroid -.github/workflows/ @Electroid +/.buildkite/ @Electroid +/.github/workflows/ @Electroid # Debugger protocol -packages/bun-inspector-protocol/ @Electroid -packages/bun-debug-adapter-protocol/ @Electroid +/packages/bun-inspector-protocol/ @Electroid +/packages/bun-debug-adapter-protocol/ @Electroid # Tests -test/expectations.txt @Jarred-Sumner +/test/expectations.txt @Jarred-Sumner diff --git a/cmake/BakeRuntimeSources.txt b/cmake/sources/BakeRuntimeSources.txt similarity index 100% rename from cmake/BakeRuntimeSources.txt rename to cmake/sources/BakeRuntimeSources.txt diff --git a/cmake/BindgenSources.txt b/cmake/sources/BindgenSources.txt similarity index 100% rename from cmake/BindgenSources.txt rename to cmake/sources/BindgenSources.txt diff --git a/cmake/BunErrorSources.txt b/cmake/sources/BunErrorSources.txt similarity index 100% rename from cmake/BunErrorSources.txt rename to cmake/sources/BunErrorSources.txt diff --git a/cmake/CSources.txt b/cmake/sources/CSources.txt similarity index 87% rename from cmake/CSources.txt rename to cmake/sources/CSources.txt index adbab642c8..67bf951f15 100644 --- a/cmake/CSources.txt +++ b/cmake/sources/CSources.txt @@ -8,5 +8,4 @@ packages/bun-usockets/src/quic.c packages/bun-usockets/src/socket.c packages/bun-usockets/src/udp.c src/bun.js/bindings/uv-posix-polyfills.c -src/bun.js/bindings/uv-posix-stubs.c -src/asan-config.c +src/bun.js/bindings/uv-posix-stubs.c \ No newline at end of file diff --git a/cmake/CxxSources.txt b/cmake/sources/CxxSources.txt similarity index 100% rename from cmake/CxxSources.txt rename to cmake/sources/CxxSources.txt diff --git a/cmake/JavaScriptCodegenSources.txt b/cmake/sources/JavaScriptCodegenSources.txt similarity index 100% rename from cmake/JavaScriptCodegenSources.txt rename to cmake/sources/JavaScriptCodegenSources.txt diff --git a/cmake/JavaScriptSources.txt b/cmake/sources/JavaScriptSources.txt similarity index 100% rename from cmake/JavaScriptSources.txt rename to cmake/sources/JavaScriptSources.txt diff --git a/cmake/NodeFallbacksSources.txt b/cmake/sources/NodeFallbacksSources.txt similarity index 100% rename from cmake/NodeFallbacksSources.txt rename to cmake/sources/NodeFallbacksSources.txt diff --git a/cmake/ZigGeneratedClassesSources.txt b/cmake/sources/ZigGeneratedClassesSources.txt similarity index 100% rename from cmake/ZigGeneratedClassesSources.txt rename to cmake/sources/ZigGeneratedClassesSources.txt diff --git a/cmake/ZigSources.txt b/cmake/sources/ZigSources.txt similarity index 99% rename from cmake/ZigSources.txt rename to cmake/sources/ZigSources.txt index 1dcd73c2a5..0aff5f740f 100644 --- a/cmake/ZigSources.txt +++ b/cmake/sources/ZigSources.txt @@ -222,6 +222,7 @@ src/bun.js/webcore/Response.zig src/bun.js/webcore/S3Client.zig src/bun.js/webcore/S3File.zig src/bun.js/webcore/S3Stat.zig +src/bun.js/webcore/ScriptExecutionContext.zig src/bun.js/webcore/Sink.zig src/bun.js/webcore/streams.zig src/bun.js/webcore/TextDecoder.zig diff --git a/cmake/targets/BuildBun.cmake b/cmake/targets/BuildBun.cmake index c9600843e7..87c2545010 100644 --- a/cmake/targets/BuildBun.cmake +++ b/cmake/targets/BuildBun.cmake @@ -46,7 +46,7 @@ endif() set(BUN_ERROR_SOURCE ${CWD}/packages/bun-error) -absolute_sources(BUN_ERROR_SOURCES ${CWD}/cmake/BunErrorSources.txt) +absolute_sources(BUN_ERROR_SOURCES ${CWD}/cmake/sources/BunErrorSources.txt) set(BUN_ERROR_OUTPUT ${CODEGEN_PATH}/bun-error) set(BUN_ERROR_OUTPUTS @@ -135,7 +135,7 @@ register_command( set(BUN_NODE_FALLBACKS_SOURCE ${CWD}/src/node-fallbacks) -absolute_sources(BUN_NODE_FALLBACKS_SOURCES ${CWD}/cmake/NodeFallbacksSources.txt) +absolute_sources(BUN_NODE_FALLBACKS_SOURCES ${CWD}/cmake/sources/NodeFallbacksSources.txt) set(BUN_NODE_FALLBACKS_OUTPUT ${CODEGEN_PATH}/node-fallbacks) set(BUN_NODE_FALLBACKS_OUTPUTS) @@ -235,7 +235,7 @@ register_command( set(BUN_ZIG_GENERATED_CLASSES_SCRIPT ${CWD}/src/codegen/generate-classes.ts) -absolute_sources(BUN_ZIG_GENERATED_CLASSES_SOURCES ${CWD}/cmake/ZigGeneratedClassesSources.txt) +absolute_sources(BUN_ZIG_GENERATED_CLASSES_SOURCES ${CWD}/cmake/sources/ZigGeneratedClassesSources.txt) set(BUN_ZIG_GENERATED_CLASSES_OUTPUTS ${CODEGEN_PATH}/ZigGeneratedClasses.h @@ -268,8 +268,8 @@ register_command( set(BUN_JAVASCRIPT_CODEGEN_SCRIPT ${CWD}/src/codegen/bundle-modules.ts) -absolute_sources(BUN_JAVASCRIPT_SOURCES ${CWD}/cmake/JavaScriptSources.txt) -absolute_sources(BUN_JAVASCRIPT_CODEGEN_SOURCES ${CWD}/cmake/JavaScriptCodegenSources.txt) +absolute_sources(BUN_JAVASCRIPT_SOURCES ${CWD}/cmake/sources/JavaScriptSources.txt) +absolute_sources(BUN_JAVASCRIPT_CODEGEN_SOURCES ${CWD}/cmake/sources/JavaScriptCodegenSources.txt) list(APPEND BUN_JAVASCRIPT_CODEGEN_SOURCES ${CWD}/src/bun.js/bindings/InternalModuleRegistry.cpp @@ -311,7 +311,7 @@ register_command( set(BUN_BAKE_RUNTIME_CODEGEN_SCRIPT ${CWD}/src/codegen/bake-codegen.ts) -absolute_sources(BUN_BAKE_RUNTIME_SOURCES ${CWD}/cmake/BakeRuntimeSources.txt) +absolute_sources(BUN_BAKE_RUNTIME_SOURCES ${CWD}/cmake/sources/BakeRuntimeSources.txt) list(APPEND BUN_BAKE_RUNTIME_CODEGEN_SOURCES ${CWD}/src/bun.js/bindings/InternalModuleRegistry.cpp @@ -344,7 +344,7 @@ register_command( set(BUN_BINDGEN_SCRIPT ${CWD}/src/codegen/bindgen.ts) -absolute_sources(BUN_BINDGEN_SOURCES ${CWD}/cmake/BindgenSources.txt) +absolute_sources(BUN_BINDGEN_SOURCES ${CWD}/cmake/sources/BindgenSources.txt) set(BUN_BINDGEN_CPP_OUTPUTS ${CODEGEN_PATH}/GeneratedBindings.cpp @@ -501,7 +501,7 @@ WEBKIT_ADD_SOURCE_DEPENDENCIES( # --- Zig --- -absolute_sources(BUN_ZIG_SOURCES ${CWD}/cmake/ZigSources.txt) +absolute_sources(BUN_ZIG_SOURCES ${CWD}/cmake/sources/ZigSources.txt) list(APPEND BUN_ZIG_SOURCES ${CWD}/build.zig @@ -598,8 +598,8 @@ set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "build.zig") set(BUN_USOCKETS_SOURCE ${CWD}/packages/bun-usockets) # hand written cpp source files. Full list of "source" code (including codegen) is in BUN_CPP_SOURCES -absolute_sources(BUN_CXX_SOURCES ${CWD}/cmake/CxxSources.txt) -absolute_sources(BUN_C_SOURCES ${CWD}/cmake/CSources.txt) +absolute_sources(BUN_CXX_SOURCES ${CWD}/cmake/sources/CxxSources.txt) +absolute_sources(BUN_C_SOURCES ${CWD}/cmake/sources/CSources.txt) if(WIN32) list(APPEND BUN_CXX_SOURCES ${CWD}/src/bun.js/bindings/windows/rescle.cpp) diff --git a/scripts/glob-sources.mjs b/scripts/glob-sources.mjs index fa20fd28ad..da991eac1c 100644 --- a/scripts/glob-sources.mjs +++ b/scripts/glob-sources.mjs @@ -23,7 +23,7 @@ async function globSources(output, patterns, excludes = []) { .sort((a, b) => a.localeCompare(b)) .join("\n"); - await write(join(root, "cmake", output), sources); + await write(join(root, "cmake", "sources", output), sources); } const input = await file(join(root, "cmake", "Sources.json")).json(); From f1504c426521a9ea95e42550c7eb132d81b0d3fd Mon Sep 17 00:00:00 2001 From: 190n Date: Tue, 20 May 2025 11:56:30 -0700 Subject: [PATCH 4/6] Add test from #18287 (#19775) --- ...alize_addon.c => async_finalize_addon.cpp} | 73 ++++++++++++++++--- test/napi/napi-app/binding.gyp | 2 +- test/napi/napi-app/module.js | 15 ++++ test/napi/napi.test.ts | 7 +- 4 files changed, 83 insertions(+), 14 deletions(-) rename test/napi/napi-app/{async_finalize_addon.c => async_finalize_addon.cpp} (54%) diff --git a/test/napi/napi-app/async_finalize_addon.c b/test/napi/napi-app/async_finalize_addon.cpp similarity index 54% rename from test/napi/napi-app/async_finalize_addon.c rename to test/napi/napi-app/async_finalize_addon.cpp index 471f9a1554..9615326dcd 100644 --- a/test/napi/napi-app/async_finalize_addon.c +++ b/test/napi/napi-app/async_finalize_addon.cpp @@ -5,14 +5,17 @@ // during a finalizer -- not during a callback scheduled with // node_api_post_finalizer -- so it cannot use NAPI_VERSION_EXPERIMENTAL. -#include +#include +#include +#include #include #include -#include -#include +#include -// "we have static_assert at home" - MSVC -char assertion[NAPI_VERSION == 8 ? 1 : -1]; +static_assert(NAPI_VERSION == 8, + "this module must be built with Node-API version 8"); + +static std::thread::id js_thread_id; #define NODE_API_CALL_CUSTOM_RETURN(env, call, retval) \ do { \ @@ -46,14 +49,14 @@ static void finalizer(napi_env env, void *data, void *hint) { (void)hint; RefHolder *ref_holder = (RefHolder *)data; NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, ref_holder->ref)); - free(ref_holder); + delete ref_holder; } static napi_value create_ref(napi_env env, napi_callback_info info) { (void)info; napi_value object; NODE_API_CALL(env, napi_create_object(env, &object)); - RefHolder *ref_holder = calloc(1, sizeof *ref_holder); + RefHolder *ref_holder = new RefHolder; NODE_API_CALL(env, napi_wrap(env, object, ref_holder, finalizer, NULL, &ref_holder->ref)); napi_value undefined; @@ -61,12 +64,58 @@ static napi_value create_ref(napi_env env, napi_callback_info info) { return undefined; } +static int buffer_finalize_count = 0; + +static void buffer_finalizer(napi_env env, void *data, void *hint) { + (void)hint; + if (std::this_thread::get_id() == js_thread_id) { + printf("buffer_finalizer run from js thread\n"); + } else { + printf("buffer_finalizer run from another thread\n"); + } + free(data); + buffer_finalize_count++; +} + +static napi_value create_buffer(napi_env env, napi_callback_info info) { + (void)info; + static const size_t len = 1000000; + void *data = malloc(len); + memset(data, 5, len); + napi_value buf; + // JavaScriptCore often runs external ArrayBuffer finalizers off the main + // thread. In this case, Bun needs to concurrently post a task to the main + // thread to invoke the finalizer. + NODE_API_CALL(env, napi_create_external_arraybuffer( + env, data, len, buffer_finalizer, NULL, &buf)); + return buf; +} + +static napi_value get_buffer_finalize_count(napi_env env, + napi_callback_info info) { + (void)info; + napi_value count; + NODE_API_CALL(env, napi_create_int32(env, buffer_finalize_count, &count)); + return count; +} + /* napi_value */ NAPI_MODULE_INIT(/* napi_env env, napi_value exports */) { - napi_value create_ref_function; + js_thread_id = std::this_thread::get_id(); + napi_value js_function; + NODE_API_CALL(env, napi_create_function(env, "create_ref", NAPI_AUTO_LENGTH, + create_ref, NULL, &js_function)); + NODE_API_CALL( + env, napi_set_named_property(env, exports, "create_ref", js_function)); NODE_API_CALL(env, - napi_create_function(env, "create_ref", NAPI_AUTO_LENGTH, - create_ref, NULL, &create_ref_function)); - NODE_API_CALL(env, napi_set_named_property(env, exports, "create_ref", - create_ref_function)); + napi_create_function(env, "create_buffer", NAPI_AUTO_LENGTH, + create_buffer, NULL, &js_function)); + NODE_API_CALL( + env, napi_set_named_property(env, exports, "create_buffer", js_function)); + NODE_API_CALL(env, napi_create_function( + env, "get_buffer_finalize_count", NAPI_AUTO_LENGTH, + get_buffer_finalize_count, NULL, &js_function)); + NODE_API_CALL(env, napi_set_named_property(env, exports, + "get_buffer_finalize_count", + js_function)); return exports; } diff --git a/test/napi/napi-app/binding.gyp b/test/napi/napi-app/binding.gyp index a7bd22a6e9..550282ed6b 100644 --- a/test/napi/napi-app/binding.gyp +++ b/test/napi/napi-app/binding.gyp @@ -81,7 +81,7 @@ }, { "target_name": "async_finalize_addon", - "sources": ["async_finalize_addon.c"], + "sources": ["async_finalize_addon.cpp"], "include_dirs": [" { } }; +nativeTests.test_external_buffer_finalizer = async () => { + const n = 50; + for (let i = 0; i < n; i++) { + let arraybuffer = asyncFinalizeAddon.create_buffer(); + let view = new Uint32Array(arraybuffer); + for (const int of view) { + // native module memsets to 5 + assert(int == 0x05050505); + } + arraybuffer = undefined; + view = undefined; + await gcUntil(() => asyncFinalizeAddon.get_buffer_finalize_count() == i + 1); + } +}; + module.exports = nativeTests; diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index c5c2169f37..480638d05b 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -369,7 +369,12 @@ describe("napi", () => { }); }); - // TODO(@190n) test allocating in a finalizer from a napi module with the right version + describe("finalizers", () => { + it("runs asynchronously on the JS thread", () => { + const out = checkSameOutput("test_external_buffer_finalizer", []); + expect(out).toMatch(/^(buffer_finalizer run from js thread\n)+resolved to undefined$/); + }); + }); describe("napi_wrap", () => { it("accepts the right kinds of values", () => { From 1ebec90d6e95000baf69d75ac76ac1804424c6cd Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Tue, 20 May 2025 12:22:01 -0700 Subject: [PATCH 5/6] Revert "Add test from #18287 (#19775)" This reverts commit f1504c426521a9ea95e42550c7eb132d81b0d3fd. --- ...alize_addon.cpp => async_finalize_addon.c} | 73 +++---------------- test/napi/napi-app/binding.gyp | 2 +- test/napi/napi-app/module.js | 15 ---- test/napi/napi.test.ts | 7 +- 4 files changed, 14 insertions(+), 83 deletions(-) rename test/napi/napi-app/{async_finalize_addon.cpp => async_finalize_addon.c} (54%) diff --git a/test/napi/napi-app/async_finalize_addon.cpp b/test/napi/napi-app/async_finalize_addon.c similarity index 54% rename from test/napi/napi-app/async_finalize_addon.cpp rename to test/napi/napi-app/async_finalize_addon.c index 9615326dcd..471f9a1554 100644 --- a/test/napi/napi-app/async_finalize_addon.cpp +++ b/test/napi/napi-app/async_finalize_addon.c @@ -5,17 +5,14 @@ // during a finalizer -- not during a callback scheduled with // node_api_post_finalizer -- so it cannot use NAPI_VERSION_EXPERIMENTAL. -#include -#include -#include +#include #include #include -#include +#include +#include -static_assert(NAPI_VERSION == 8, - "this module must be built with Node-API version 8"); - -static std::thread::id js_thread_id; +// "we have static_assert at home" - MSVC +char assertion[NAPI_VERSION == 8 ? 1 : -1]; #define NODE_API_CALL_CUSTOM_RETURN(env, call, retval) \ do { \ @@ -49,14 +46,14 @@ static void finalizer(napi_env env, void *data, void *hint) { (void)hint; RefHolder *ref_holder = (RefHolder *)data; NODE_API_CALL_RETURN_VOID(env, napi_delete_reference(env, ref_holder->ref)); - delete ref_holder; + free(ref_holder); } static napi_value create_ref(napi_env env, napi_callback_info info) { (void)info; napi_value object; NODE_API_CALL(env, napi_create_object(env, &object)); - RefHolder *ref_holder = new RefHolder; + RefHolder *ref_holder = calloc(1, sizeof *ref_holder); NODE_API_CALL(env, napi_wrap(env, object, ref_holder, finalizer, NULL, &ref_holder->ref)); napi_value undefined; @@ -64,58 +61,12 @@ static napi_value create_ref(napi_env env, napi_callback_info info) { return undefined; } -static int buffer_finalize_count = 0; - -static void buffer_finalizer(napi_env env, void *data, void *hint) { - (void)hint; - if (std::this_thread::get_id() == js_thread_id) { - printf("buffer_finalizer run from js thread\n"); - } else { - printf("buffer_finalizer run from another thread\n"); - } - free(data); - buffer_finalize_count++; -} - -static napi_value create_buffer(napi_env env, napi_callback_info info) { - (void)info; - static const size_t len = 1000000; - void *data = malloc(len); - memset(data, 5, len); - napi_value buf; - // JavaScriptCore often runs external ArrayBuffer finalizers off the main - // thread. In this case, Bun needs to concurrently post a task to the main - // thread to invoke the finalizer. - NODE_API_CALL(env, napi_create_external_arraybuffer( - env, data, len, buffer_finalizer, NULL, &buf)); - return buf; -} - -static napi_value get_buffer_finalize_count(napi_env env, - napi_callback_info info) { - (void)info; - napi_value count; - NODE_API_CALL(env, napi_create_int32(env, buffer_finalize_count, &count)); - return count; -} - /* napi_value */ NAPI_MODULE_INIT(/* napi_env env, napi_value exports */) { - js_thread_id = std::this_thread::get_id(); - napi_value js_function; - NODE_API_CALL(env, napi_create_function(env, "create_ref", NAPI_AUTO_LENGTH, - create_ref, NULL, &js_function)); - NODE_API_CALL( - env, napi_set_named_property(env, exports, "create_ref", js_function)); + napi_value create_ref_function; NODE_API_CALL(env, - napi_create_function(env, "create_buffer", NAPI_AUTO_LENGTH, - create_buffer, NULL, &js_function)); - NODE_API_CALL( - env, napi_set_named_property(env, exports, "create_buffer", js_function)); - NODE_API_CALL(env, napi_create_function( - env, "get_buffer_finalize_count", NAPI_AUTO_LENGTH, - get_buffer_finalize_count, NULL, &js_function)); - NODE_API_CALL(env, napi_set_named_property(env, exports, - "get_buffer_finalize_count", - js_function)); + napi_create_function(env, "create_ref", NAPI_AUTO_LENGTH, + create_ref, NULL, &create_ref_function)); + NODE_API_CALL(env, napi_set_named_property(env, exports, "create_ref", + create_ref_function)); return exports; } diff --git a/test/napi/napi-app/binding.gyp b/test/napi/napi-app/binding.gyp index 550282ed6b..a7bd22a6e9 100644 --- a/test/napi/napi-app/binding.gyp +++ b/test/napi/napi-app/binding.gyp @@ -81,7 +81,7 @@ }, { "target_name": "async_finalize_addon", - "sources": ["async_finalize_addon.cpp"], + "sources": ["async_finalize_addon.c"], "include_dirs": [" { } }; -nativeTests.test_external_buffer_finalizer = async () => { - const n = 50; - for (let i = 0; i < n; i++) { - let arraybuffer = asyncFinalizeAddon.create_buffer(); - let view = new Uint32Array(arraybuffer); - for (const int of view) { - // native module memsets to 5 - assert(int == 0x05050505); - } - arraybuffer = undefined; - view = undefined; - await gcUntil(() => asyncFinalizeAddon.get_buffer_finalize_count() == i + 1); - } -}; - module.exports = nativeTests; diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index 480638d05b..c5c2169f37 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -369,12 +369,7 @@ describe("napi", () => { }); }); - describe("finalizers", () => { - it("runs asynchronously on the JS thread", () => { - const out = checkSameOutput("test_external_buffer_finalizer", []); - expect(out).toMatch(/^(buffer_finalizer run from js thread\n)+resolved to undefined$/); - }); - }); + // TODO(@190n) test allocating in a finalizer from a napi module with the right version describe("napi_wrap", () => { it("accepts the right kinds of values", () => { From d1ac52da2c71e6ba67fc4796d4dcc3d3c1657c10 Mon Sep 17 00:00:00 2001 From: 190n Date: Tue, 20 May 2025 12:41:06 -0700 Subject: [PATCH 6/6] ci: use ARM EC2 instances for build-zig (#19781) --- .buildkite/ci.mjs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.buildkite/ci.mjs b/.buildkite/ci.mjs index c0ac6b0673..8aa1460057 100755 --- a/.buildkite/ci.mjs +++ b/.buildkite/ci.mjs @@ -331,16 +331,14 @@ function getZigAgent(platform, options) { return getEc2Agent( { os: "linux", - arch: "x64", + arch: "aarch64", abi: "musl", distro: "alpine", release: "3.21", }, options, { - instanceType: "c7i.2xlarge", - cpuCount: 4, - threadsPerCore: 1, + instanceType: "r8g.large", }, ); }