From 524bccdf6aa20ee4ba76dc7291d77b4c98fa7e28 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 1 Feb 2023 06:41:04 -0800 Subject: [PATCH] fix(napi): return node globalThis from napi_get_global (#17613) Fixes https://github.com/denoland/deno/issues/17587 --- cli/napi/js_native_api.rs | 8 ++++---- ext/napi/lib.rs | 5 +++++ ext/node/02_require.js | 2 +- test_napi/common.js | 6 +++++- test_napi/env_test.js | 12 ++++++++++++ test_napi/src/env.rs | 39 +++++++++++++++++++++++++++++++++++++++ test_napi/src/lib.rs | 2 ++ 7 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 test_napi/env_test.js create mode 100644 test_napi/src/env.rs diff --git a/cli/napi/js_native_api.rs b/cli/napi/js_native_api.rs index 62ac40159d..aefe8dd0d9 100644 --- a/cli/napi/js_native_api.rs +++ b/cli/napi/js_native_api.rs @@ -1718,12 +1718,12 @@ fn napi_get_element( #[napi_sym::napi_sym] fn napi_get_global(env: *mut Env, result: *mut napi_value) -> Result { check_env!(env); - let env = unsafe { &mut *env }; + check_arg!(env, result); - let context = &mut env.scope().get_current_context(); - let global = context.global(&mut env.scope()); - let value: v8::Local = global.into(); + let value: v8::Local = + transmute::, v8::Local>((*env).global); *result = value.into(); + napi_clear_last_error(env); Ok(()) } diff --git a/ext/napi/lib.rs b/ext/napi/lib.rs index 789aec5eb7..59a07136df 100644 --- a/ext/napi/lib.rs +++ b/ext/napi/lib.rs @@ -418,6 +418,7 @@ pub struct Env { Rc>>, pub tsfn_ref_counters: Arc>, pub last_error: napi_extended_error_info, + pub global: NonNull, } unsafe impl Send for Env {} @@ -427,6 +428,7 @@ impl Env { pub fn new( isolate_ptr: *mut v8::OwnedIsolate, context: v8::Global, + global: v8::Global, sender: mpsc::UnboundedSender, threadsafe_function_sender: mpsc::UnboundedSender, cleanup_hooks: Rc< @@ -446,6 +448,7 @@ impl Env { Self { isolate_ptr, context: context.into_raw(), + global: global.into_raw(), shared: std::ptr::null_mut(), open_handle_scopes: 0, async_work_sender: sender, @@ -602,6 +605,7 @@ fn op_napi_open( scope: &mut v8::HandleScope<'scope>, op_state: &mut OpState, path: String, + global: serde_v8::Value, ) -> std::result::Result, AnyError> where NP: NapiPermissions + 'static, @@ -644,6 +648,7 @@ where let mut env = Env::new( isolate_ptr, v8::Global::new(scope, ctx), + v8::Global::new(scope, global.v8_value), async_work_sender, tsfn_sender, cleanup_hooks, diff --git a/ext/node/02_require.js b/ext/node/02_require.js index 7b44787fd7..bda74c01fc 100644 --- a/ext/node/02_require.js +++ b/ext/node/02_require.js @@ -804,7 +804,7 @@ if (filename.endsWith("fsevents.node")) { throw new Error("Using fsevents module is currently not supported"); } - module.exports = ops.op_napi_open(filename); + module.exports = ops.op_napi_open(filename, node.globalThis); }; function createRequireFromPath(filename) { diff --git a/test_napi/common.js b/test_napi/common.js index 915a43818e..09378918f1 100644 --- a/test_napi/common.js +++ b/test_napi/common.js @@ -17,5 +17,9 @@ const [libPrefix, libSuffix] = { export function loadTestLibrary() { const specifier = `${targetDir}/${libPrefix}test_napi.${libSuffix}`; - return Deno[Deno.internal].core.ops.op_napi_open(specifier); // Internal, used in ext/node + + // Internal, used in ext/node + return Deno[Deno.internal].core.ops.op_napi_open(specifier, { + Buffer: {}, + }); } diff --git a/test_napi/env_test.js b/test_napi/env_test.js new file mode 100644 index 0000000000..8ec12a5340 --- /dev/null +++ b/test_napi/env_test.js @@ -0,0 +1,12 @@ +// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. + +import { assert, loadTestLibrary } from "./common.js"; + +const env = loadTestLibrary(); + +Deno.test("napi get global", function () { + const g = env.testNodeGlobal(); + // Note: global is a mock object in the tests. + // See common.js + assert(g.Buffer); +}); diff --git a/test_napi/src/env.rs b/test_napi/src/env.rs new file mode 100644 index 0000000000..620aeec297 --- /dev/null +++ b/test_napi/src/env.rs @@ -0,0 +1,39 @@ +// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. + +use crate::assert_napi_ok; +use crate::napi_get_callback_info; +use crate::napi_new_property; +use napi_sys::*; + +extern "C" fn get_node_global( + env: napi_env, + info: napi_callback_info, +) -> napi_value { + let (_, argc, _) = napi_get_callback_info!(env, info, 0); + assert_eq!(argc, 0); + + let mut result: napi_value = std::ptr::null_mut(); + assert_napi_ok!(napi_get_global(env, &mut result)); + + let mut r1: napi_value = std::ptr::null_mut(); + assert_napi_ok!(napi_get_named_property( + env, + result, + "Buffer\0".as_ptr() as _, + &mut r1 + )); + + result +} + +pub fn init(env: napi_env, exports: napi_value) { + let properties = + &[napi_new_property!(env, "testNodeGlobal", get_node_global)]; + + assert_napi_ok!(napi_define_properties( + env, + exports, + properties.len(), + properties.as_ptr() + )); +} diff --git a/test_napi/src/lib.rs b/test_napi/src/lib.rs index 71258ee7fc..ed0afb741b 100644 --- a/test_napi/src/lib.rs +++ b/test_napi/src/lib.rs @@ -12,6 +12,7 @@ pub mod r#async; pub mod callback; pub mod coerce; pub mod date; +pub mod env; pub mod error; pub mod mem; pub mod numbers; @@ -143,6 +144,7 @@ unsafe extern "C" fn napi_register_module_v1( typedarray::init(env, exports); arraybuffer::init(env, exports); array::init(env, exports); + env::init(env, exports); error::init(env, exports); primitives::init(env, exports); properties::init(env, exports);