diff --git a/.github/workflows/ci.generate.ts b/.github/workflows/ci.generate.ts index c1f1eef6b4..44fd3c47c7 100755 --- a/.github/workflows/ci.generate.ts +++ b/.github/workflows/ci.generate.ts @@ -17,7 +17,7 @@ const Runners = (() => { })(); // bump the number at the start when you want to purge the cache const prCacheKeyPrefix = - "29-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-${{ matrix.job }}-"; + "31-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-${{ matrix.job }}-"; const installPkgsCommand = "sudo apt-get install --no-install-recommends debootstrap clang-15 lld-15"; @@ -480,7 +480,7 @@ const ci = { "~/.cargo/git/db", ].join("\n"), key: - "29-cargo-home-${{ matrix.os }}-${{ hashFiles('Cargo.lock') }}", + "31-cargo-home-${{ matrix.os }}-${{ hashFiles('Cargo.lock') }}", }, }, { diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c78791ee5b..97219b4d0c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -293,7 +293,7 @@ jobs: ~/.cargo/registry/index ~/.cargo/registry/cache ~/.cargo/git/db - key: '29-cargo-home-${{ matrix.os }}-${{ hashFiles(''Cargo.lock'') }}' + key: '31-cargo-home-${{ matrix.os }}-${{ hashFiles(''Cargo.lock'') }}' if: '!(github.event_name == ''pull_request'' && matrix.skip_pr)' - name: Restore cache build output (PR) uses: actions/cache/restore@v3 @@ -305,7 +305,7 @@ jobs: !./target/*/*.zip !./target/*/*.tar.gz key: never_saved - restore-keys: '29-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-${{ matrix.job }}-' + restore-keys: '31-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-${{ matrix.job }}-' - name: Apply and update mtime cache if: '!(github.event_name == ''pull_request'' && matrix.skip_pr) && (!startsWith(github.ref, ''refs/tags/''))' uses: ./.github/mtime_cache @@ -589,7 +589,7 @@ jobs: !./target/*/gn_out !./target/*/*.zip !./target/*/*.tar.gz - key: '29-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-${{ matrix.job }}-${{ github.sha }}' + key: '31-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-${{ matrix.job }}-${{ github.sha }}' publish-canary: name: publish canary runs-on: ubuntu-22.04 diff --git a/cli/napi/env.rs b/cli/napi/env.rs index 114c43026f..decdd59d6f 100644 --- a/cli/napi/env.rs +++ b/cli/napi/env.rs @@ -136,9 +136,10 @@ fn node_api_get_module_file_name( #[napi_sym::napi_sym] fn napi_module_register(module: *const NapiModule) -> napi_status { - MODULE.with(|cell| { + MODULE_TO_REGISTER.with(|cell| { let mut slot = cell.borrow_mut(); - slot.replace(module); + let prev = slot.replace(module); + assert!(prev.is_none()); }); napi_ok } diff --git a/ext/napi/lib.rs b/ext/napi/lib.rs index 32f8c52fc5..22d86e4a9b 100644 --- a/ext/napi/lib.rs +++ b/ext/napi/lib.rs @@ -81,9 +81,7 @@ pub const napi_would_deadlock: napi_status = 21; pub const NAPI_AUTO_LENGTH: usize = usize::MAX; thread_local! { - pub static MODULE: RefCell> = RefCell::new(None); - pub static ASYNC_WORK_SENDER: RefCell>> = RefCell::new(None); - pub static THREAD_SAFE_FN_SENDER: RefCell>> = RefCell::new(None); + pub static MODULE_TO_REGISTER: RefCell> = RefCell::new(None); } type napi_addon_register_func = @@ -346,15 +344,6 @@ impl Env { >, tsfn_ref_counters: Arc>, ) -> Self { - let sc = sender.clone(); - ASYNC_WORK_SENDER.with(|s| { - s.replace(Some(sc)); - }); - let ts = threadsafe_function_sender.clone(); - THREAD_SAFE_FN_SENDER.with(|s| { - s.replace(Some(ts)); - }); - Self { isolate_ptr, context: context.into_raw(), @@ -559,7 +548,6 @@ where { let permissions = op_state.borrow_mut::(); permissions.check(Some(&PathBuf::from(&path)))?; - let ( async_work_sender, tsfn_sender, @@ -622,77 +610,67 @@ where Err(e) => return Err(type_error(e.to_string())), }; - MODULE.with(|cell| { - let slot = *cell.borrow(); - let obj = match slot { - Some(nm) => { - // SAFETY: napi_register_module guarantees that `nm` is valid. - let nm = unsafe { &*nm }; - assert_eq!(nm.nm_version, 1); - // SAFETY: we are going blind, calling the register function on the other side. - let maybe_exports = unsafe { - (nm.nm_register_func)( - env_ptr, - std::mem::transmute::, napi_value>( - exports.into(), - ), - ) - }; + let maybe_module = MODULE_TO_REGISTER.with(|cell| { + let mut slot = cell.borrow_mut(); + slot.take() + }); - let exports = maybe_exports - .as_ref() - .map(|_| unsafe { - // SAFETY: v8::Local is a pointer to a value and napi_value is also a pointer - // to a value, they have the same layout - std::mem::transmute::>( - maybe_exports, - ) - }) - .unwrap_or_else(|| { - // If the module didn't return anything, we use the exports object. - exports.into() - }); - - Ok(serde_v8::Value { v8_value: exports }) - } - None => { - // Initializer callback. - // SAFETY: we are going blind, calling the register function on the other side. - unsafe { - let init = library - .get:: napi_value>(b"napi_register_module_v1") - .expect("napi_register_module_v1 not found"); - let maybe_exports = init( - env_ptr, - std::mem::transmute::, napi_value>( - exports.into(), - ), - ); - - let exports = maybe_exports - .as_ref() - .map(|_| { - // SAFETY: v8::Local is a pointer to a value and napi_value is also a pointer - // to a value, they have the same layout - std::mem::transmute::>( - maybe_exports, - ) - }) - .unwrap_or_else(|| { - // If the module didn't return anything, we use the exports object. - exports.into() - }); - - Ok(serde_v8::Value { v8_value: exports }) - } - } + if let Some(module_to_register) = maybe_module { + // SAFETY: napi_register_module guarantees that `module_to_register` is valid. + let nm = unsafe { &*module_to_register }; + assert_eq!(nm.nm_version, 1); + // SAFETY: we are going blind, calling the register function on the other side. + let maybe_exports = unsafe { + (nm.nm_register_func)( + env_ptr, + std::mem::transmute::, napi_value>(exports.into()), + ) }; + + let exports = if maybe_exports.is_some() { + // SAFETY: v8::Local is a pointer to a value and napi_value is also a pointer + // to a value, they have the same layout + unsafe { + std::mem::transmute::>(maybe_exports) + } + } else { + exports.into() + }; + // NAPI addons can't be unloaded, so we're going to "forget" the library // object so it lives till the program exit. std::mem::forget(library); - obj - }) + return Ok(serde_v8::Value { v8_value: exports }); + } + + // Initializer callback. + // SAFETY: we are going blind, calling the register function on the other side. + + let maybe_exports = unsafe { + let init = library + .get:: napi_value>(b"napi_register_module_v1") + .expect("napi_register_module_v1 not found"); + init( + env_ptr, + std::mem::transmute::, napi_value>(exports.into()), + ) + }; + + let exports = if maybe_exports.is_some() { + // SAFETY: v8::Local is a pointer to a value and napi_value is also a pointer + // to a value, they have the same layout + unsafe { + std::mem::transmute::>(maybe_exports) + } + } else { + exports.into() + }; + + // NAPI addons can't be unloaded, so we're going to "forget" the library + // object so it lives till the program exit. + std::mem::forget(library); + Ok(serde_v8::Value { v8_value: exports }) } diff --git a/test_napi/.gitignore b/test_napi/.gitignore index 6fdcc4a662..54de1ef345 100644 --- a/test_napi/.gitignore +++ b/test_napi/.gitignore @@ -1,4 +1,7 @@ package-lock.json # Test generated artifacts -.swc \ No newline at end of file +.swc +*.dylib +*.so +*.dll diff --git a/test_napi/common.js b/test_napi/common.js index 09378918f1..ce9b2544b1 100644 --- a/test_napi/common.js +++ b/test_napi/common.js @@ -9,17 +9,19 @@ export { export { fromFileUrl } from "../test_util/std/path/mod.ts"; const targetDir = Deno.execPath().replace(/[^\/\\]+$/, ""); -const [libPrefix, libSuffix] = { +export const [libPrefix, libSuffix] = { darwin: ["lib", "dylib"], linux: ["lib", "so"], windows: ["", "dll"], }[Deno.build.os]; +const ops = Deno[Deno.internal].core.ops; + export function loadTestLibrary() { const specifier = `${targetDir}/${libPrefix}test_napi.${libSuffix}`; // Internal, used in ext/node - return Deno[Deno.internal].core.ops.op_napi_open(specifier, { + return ops.op_napi_open(specifier, { Buffer: {}, }); } diff --git a/test_napi/init_test.js b/test_napi/init_test.js new file mode 100644 index 0000000000..633fdbb615 --- /dev/null +++ b/test_napi/init_test.js @@ -0,0 +1,14 @@ +// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. + +import { assert, libSuffix } from "./common.js"; + +const ops = Deno[Deno.internal].core.ops; + +Deno.test("ctr initialization (napi_module_register)", { + ignore: Deno.build.os == "windows", +}, function () { + const path = new URL(`./module.${libSuffix}`, import.meta.url).pathname; + const obj = ops.op_napi_open(path, {}); + assert(obj != null); + assert(typeof obj === "object"); +}); diff --git a/test_napi/module.c b/test_napi/module.c new file mode 100644 index 0000000000..4548aa37fb --- /dev/null +++ b/test_napi/module.c @@ -0,0 +1,68 @@ +typedef struct napi_module { + int nm_version; + unsigned int nm_flags; + const char* nm_filename; + void* nm_register_func; + const char* nm_modname; + void* nm_priv; + void* reserved[4]; +} napi_module; + +#ifdef _WIN32 +#define NAPI_EXTERN __declspec(dllexport) +#define NAPI_CDECL __cdecl +#else +#define NAPI_EXTERN __attribute__((visibility("default"))) +#define NAPI_CDECL +#endif + +NAPI_EXTERN void NAPI_CDECL +napi_module_register(napi_module* mod); + +#if defined(_MSC_VER) +#if defined(__cplusplus) +#define NAPI_C_CTOR(fn) \ + static void NAPI_CDECL fn(void); \ + namespace { \ + struct fn##_ { \ + fn##_() { fn(); } \ + } fn##_v_; \ + } \ + static void NAPI_CDECL fn(void) +#else // !defined(__cplusplus) +#pragma section(".CRT$XCU", read) +// The NAPI_C_CTOR macro defines a function fn that is called during CRT +// initialization. +// C does not support dynamic initialization of static variables and this code +// simulates C++ behavior. Exporting the function pointer prevents it from being +// optimized. See for details: +// https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-initialization?view=msvc-170 +#define NAPI_C_CTOR(fn) \ + static void NAPI_CDECL fn(void); \ + __declspec(dllexport, allocate(".CRT$XCU")) void(NAPI_CDECL * fn##_)(void) = \ + fn; \ + static void NAPI_CDECL fn(void) +#endif // defined(__cplusplus) +#else +#define NAPI_C_CTOR(fn) \ + static void fn(void) __attribute__((constructor)); \ + static void fn(void) +#endif + +#define NAPI_MODULE_TEST(modname, regfunc) \ + static napi_module _module = { \ + 1, \ + 0, \ + __FILE__, \ + regfunc, \ + #modname, \ + 0, \ + {0}, \ + }; \ + NAPI_C_CTOR(_register_##modname) { napi_module_register(&_module); } \ + +void* init(void* env __attribute__((unused)), void* exports) { + return exports; +} + +NAPI_MODULE_TEST(TEST_NAPI_MODULE_NAME, init) diff --git a/test_napi/tests/napi_tests.rs b/test_napi/tests/napi_tests.rs index 3e39894361..c3ce285e07 100644 --- a/test_napi/tests/napi_tests.rs +++ b/test_napi/tests/napi_tests.rs @@ -18,6 +18,35 @@ fn build() { } let build_plugin_output = build_plugin.output().unwrap(); assert!(build_plugin_output.status.success()); + + // cc module.c -undefined dynamic_lookup -shared -Wl,-no_fixup_chains -dynamic -o module.dylib + #[cfg(not(target_os = "windows"))] + { + let out = if cfg!(target_os = "macos") { + "module.dylib" + } else { + "module.so" + }; + + let mut cc = Command::new("cc"); + + #[cfg(not(target_os = "macos"))] + let c_module = cc.arg("module.c").arg("-shared").arg("-o").arg(out); + + #[cfg(target_os = "macos")] + let c_module = { + cc.arg("module.c") + .arg("-undefined") + .arg("dynamic_lookup") + .arg("-shared") + .arg("-Wl,-no_fixup_chains") + .arg("-dynamic") + .arg("-o") + .arg(out) + }; + let c_module_output = c_module.output().unwrap(); + assert!(c_module_output.status.success()); + } } #[test]