mirror of
https://github.com/denoland/deno.git
synced 2024-12-21 23:04:45 -05:00
fix(napi): Fix worker threads importing already-loaded NAPI addon (#25245)
Part of #20613. If a node addon is using the legacy `napi_module_register` on ctor approach to module registration, we have to store the registered module so that other threads can load the addon (because `napi_module_register` will only be called once per process).
This commit is contained in:
parent
0e50bb1d4a
commit
7dd861aa36
2 changed files with 58 additions and 0 deletions
|
@ -9,11 +9,13 @@ use core::ptr::NonNull;
|
|||
use deno_core::error::type_error;
|
||||
use deno_core::error::AnyError;
|
||||
use deno_core::op2;
|
||||
use deno_core::parking_lot::RwLock;
|
||||
use deno_core::url::Url;
|
||||
use deno_core::ExternalOpsTracker;
|
||||
use deno_core::OpState;
|
||||
use deno_core::V8CrossThreadTaskSpawner;
|
||||
use std::cell::RefCell;
|
||||
use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::rc::Rc;
|
||||
|
@ -493,6 +495,15 @@ impl NapiPermissions for deno_permissions::PermissionsContainer {
|
|||
}
|
||||
}
|
||||
|
||||
unsafe impl Sync for NapiModuleHandle {}
|
||||
unsafe impl Send for NapiModuleHandle {}
|
||||
#[derive(Clone, Copy)]
|
||||
struct NapiModuleHandle(*const NapiModule);
|
||||
|
||||
static NAPI_LOADED_MODULES: std::sync::LazyLock<
|
||||
RwLock<HashMap<String, NapiModuleHandle>>,
|
||||
> = std::sync::LazyLock::new(|| RwLock::new(HashMap::new()));
|
||||
|
||||
#[op2(reentrant)]
|
||||
fn op_napi_open<NP, 'scope>(
|
||||
scope: &mut v8::HandleScope<'scope>,
|
||||
|
@ -575,11 +586,23 @@ where
|
|||
let exports = v8::Object::new(scope);
|
||||
|
||||
let maybe_exports = if let Some(module_to_register) = maybe_module {
|
||||
NAPI_LOADED_MODULES
|
||||
.write()
|
||||
.insert(path, NapiModuleHandle(module_to_register));
|
||||
// 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.
|
||||
unsafe { (nm.nm_register_func)(env_ptr, exports.into()) }
|
||||
} else if let Some(module_to_register) =
|
||||
{ NAPI_LOADED_MODULES.read().get(&path).copied() }
|
||||
{
|
||||
// SAFETY: this originated from `napi_register_module`, so the
|
||||
// pointer should still be valid.
|
||||
let nm = unsafe { &*module_to_register.0 };
|
||||
assert_eq!(nm.nm_version, 1);
|
||||
// SAFETY: we are going blind, calling the register function on the other side.
|
||||
unsafe { (nm.nm_register_func)(env_ptr, exports.into()) }
|
||||
} else if let Ok(init) = unsafe {
|
||||
library.get::<napi_register_module_v1>(b"napi_register_module_v1")
|
||||
} {
|
||||
|
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
import { Buffer } from "node:buffer";
|
||||
import { assert, libSuffix } from "./common.js";
|
||||
import { Worker } from "node:worker_threads";
|
||||
|
||||
const ops = Deno[Deno.internal].core.ops;
|
||||
|
||||
|
@ -13,3 +14,37 @@ Deno.test("ctr initialization (napi_module_register)", {
|
|||
assert(obj != null);
|
||||
assert(typeof obj === "object");
|
||||
});
|
||||
|
||||
Deno.test("ctr initialization by multiple threads (napi_module_register)", {
|
||||
ignore: Deno.build.os == "windows",
|
||||
}, async function () {
|
||||
const path = new URL(`./module.${libSuffix}`, import.meta.url).pathname;
|
||||
const obj = ops.op_napi_open(path, {}, Buffer, reportError);
|
||||
const common = import.meta.resolve("./common.js");
|
||||
assert(obj != null);
|
||||
assert(typeof obj === "object");
|
||||
|
||||
const worker = new Worker(
|
||||
`
|
||||
import { Buffer } from "node:buffer";
|
||||
import { parentPort } from "node:worker_threads";
|
||||
import { assert } from "${common}";
|
||||
|
||||
const ops = Deno[Deno.internal].core.ops;
|
||||
const obj = ops.op_napi_open("${path}", {}, Buffer, reportError);
|
||||
assert(obj != null);
|
||||
assert(typeof obj === "object");
|
||||
parentPort.postMessage("ok");
|
||||
`,
|
||||
{
|
||||
eval: true,
|
||||
},
|
||||
);
|
||||
|
||||
const p = Promise.withResolvers();
|
||||
worker.on("message", (_m) => {
|
||||
p.resolve();
|
||||
});
|
||||
|
||||
await p.promise;
|
||||
});
|
||||
|
|
Loading…
Reference in a new issue