Reimplement Module::ResolveCallback ABI fix without global variables (#207)

src/binding.cc

@@ -1353,30 +1353,10 @@ int v8__Module__GetIdentityHash(const v8::Module& self) {
   return self.GetIdentityHash();
 }
 
-// This is an extern C calling convention compatible version of
-// v8::Module::ResolveCallback.
-using v8__Module__ResolveCallback =
-    const v8::Module* (*)(v8::Local<v8::Context> context,
-                          v8::Local<v8::String> specifier,
-                          v8::Local<v8::Module> referrer);
-
 MaybeBool v8__Module__InstantiateModule(v8::Module& self,
                                         v8::Local<v8::Context> context,
-                                        v8__Module__ResolveCallback c_cb) {
-  thread_local v8__Module__ResolveCallback static_cb = nullptr;
-  assert(static_cb == nullptr);
-  static_cb = c_cb;
-
-  auto cxx_cb = [](v8::Local<v8::Context> context,
-                   v8::Local<v8::String> specifier,
-                   v8::Local<v8::Module> referrer) {
-    const auto* m = static_cb(context, specifier, referrer);
-    return ptr_to_maybe_local(const_cast<v8::Module*>(m));
-  };
-
-  auto r = maybe_to_maybe_bool(self.InstantiateModule(context, cxx_cb));
-  static_cb = nullptr;
-  return r;
+                                        v8::Module::ResolveCallback cb) {
+  return maybe_to_maybe_bool(self.InstantiateModule(context, cb));
 }
 
 v8::Value* v8__Module__Evaluate(v8::Module& self,

src/module.rs

@@ -1,19 +1,86 @@
+use std::mem::MaybeUninit;
+use std::ptr::null;
+
 use crate::support::int;
+use crate::support::MapFnFrom;
+use crate::support::MapFnTo;
 use crate::support::MaybeBool;
+use crate::support::ToCFn;
+use crate::support::UnitType;
 use crate::Context;
 use crate::Local;
 use crate::Module;
 use crate::String;
 use crate::ToLocal;
 use crate::Value;
-use std::mem::MaybeUninit;
 
 /// Called during Module::instantiate_module. Provided with arguments:
-/// (context, specifier, referrer)
-/// Return null on error.
-/// Hint: to tranform Local<Module> to *const Module use `&*module`.
-pub type ResolveCallback =
-  extern "C" fn(Local<Context>, Local<String>, Local<Module>) -> *const Module;
+/// (context, specifier, referrer). Return None on error.
+///
+/// Note: this callback has an unusual signature due to ABI incompatibilities
+/// between Rust and C++. However end users can implement the callback as
+/// follows; it'll be automatically converted.
+///
+/// ```rust,ignore
+///   fn my_resolve_callback<'a>(
+///      context: v8::Local<'a, v8::Context>,
+///      specifier: v8::Local<'a, v8::String>,
+///      referrer: v8::Local<'a, v8::Module>,
+///   ) -> Option<v8::Local<'a, v8::Module>> {
+///      // ...
+///      Some(resolved_module)
+///   }
+/// ```
+///
+
+// System V AMD64 ABI: Local<Module> returned in a register.
+#[cfg(not(target_os = "windows"))]
+pub type ResolveCallback<'a> = extern "C" fn(
+  Local<'a, Context>,
+  Local<'a, String>,
+  Local<'a, Module>,
+) -> *const Module;
+
+// Windows x64 ABI: Local<Module> returned on the stack.
+#[cfg(target_os = "windows")]
+pub type ResolveCallback<'a> = extern "C" fn(
+  *mut *const Module,
+  Local<'a, Context>,
+  Local<'a, String>,
+  Local<'a, Module>,
+) -> *mut *const Module;
+
+impl<'a, F> MapFnFrom<F> for ResolveCallback<'a>
+where
+  F: UnitType
+    + Fn(
+      Local<'a, Context>,
+      Local<'a, String>,
+      Local<'a, Module>,
+    ) -> Option<Local<'a, Module>>,
+{
+  #[cfg(not(target_os = "windows"))]
+  fn mapping() -> Self {
+    let f = |context, specifier, referrer| {
+      (F::get())(context, specifier, referrer)
+        .map(|r| -> *const Module { &*r })
+        .unwrap_or(null())
+    };
+    f.to_c_fn()
+  }
+
+  #[cfg(target_os = "windows")]
+  fn mapping() -> Self {
+    let f = |ret_ptr, context, specifier, referrer| {
+      let r = (F::get())(context, specifier, referrer)
+        .map(|r| -> *const Module { &*r })
+        .unwrap_or(null());
+      unsafe { std::ptr::write(ret_ptr, r) }; // Write result to stack.
+      ret_ptr // Return stack pointer to the return value.
+    };
+    f.to_c_fn()
+  }
+}
 
 extern "C" {
   fn v8__Module__GetStatus(this: *const Module) -> ModuleStatus;
@@ -120,12 +187,15 @@ impl Module {
   /// instantiation. (In the case where the callback throws an exception, that
   /// exception is propagated.)
   #[must_use]
-  pub fn instantiate_module(
+  pub fn instantiate_module<'a>(
     &mut self,
     context: Local<Context>,
-    callback: ResolveCallback,
+    callback: impl MapFnTo<ResolveCallback<'a>>,
   ) -> Option<bool> {
-    unsafe { v8__Module__InstantiateModule(self, context, callback) }.into()
+    unsafe {
+      v8__Module__InstantiateModule(self, context, callback.map_fn_to())
+    }
+    .into()
   }
 
   /// Evaluates the module and its dependencies.

src/support.rs

@@ -15,7 +15,7 @@ pub use std::os::raw::c_char as char;
 pub use std::os::raw::c_int as int;
 pub use std::os::raw::c_long as long;
 
-pub type Opaque = [usize; 0];
+pub type Opaque = [u8; 0];
 
 pub trait Delete
 where
@@ -273,3 +273,165 @@ impl<T> Into<Option<T>> for Maybe<T> {
     }
   }
 }
+
+pub trait UnitType
+where
+  Self: Copy + Sized,
+{
+  #[inline(always)]
+  fn get() -> Self {
+    UnitValue::<Self>::get()
+  }
+}
+
+impl<T> UnitType for T where T: Copy + Sized {}
+
+#[derive(Copy, Clone)]
+struct UnitValue<T>(PhantomData<T>)
+where
+  Self: Sized;
+
+impl<T> UnitValue<T>
+where
+  Self: Copy + Sized,
+{
+  const SELF: Self = Self::new_checked();
+
+  const fn new_checked() -> Self {
+    // Statically assert that T is indeed a unit type.
+    let size_must_be_0 = size_of::<T>();
+    let s = Self(PhantomData::<T>);
+    [s][size_must_be_0]
+  }
+
+  #[inline(always)]
+  fn get_checked(self) -> T {
+    // This run-time check serves just as a backup for the compile-time
+    // check when Self::SELF is initialized.
+    assert_eq!(size_of::<T>(), 0);
+    unsafe { std::mem::MaybeUninit::<T>::zeroed().assume_init() }
+  }
+
+  #[inline(always)]
+  pub fn get() -> T {
+    // Accessing the Self::SELF is necessary to make the compile-time type check
+    // work.
+    Self::SELF.get_checked()
+  }
+}
+
+pub struct DefaultTag;
+pub struct IdenticalConversionTag;
+
+pub trait MapFnFrom<F, Tag = DefaultTag>
+where
+  F: UnitType,
+  Self: Sized,
+{
+  fn mapping() -> Self;
+
+  #[inline(always)]
+  fn map_fn_from(_: F) -> Self {
+    Self::mapping()
+  }
+}
+
+impl<F> MapFnFrom<F, IdenticalConversionTag> for F
+where
+  Self: UnitType,
+{
+  #[inline(always)]
+  fn mapping() -> Self {
+    Self::get()
+  }
+}
+
+pub trait MapFnTo<T, Tag = DefaultTag>
+where
+  Self: UnitType,
+  T: Sized,
+{
+  fn mapping() -> T;
+
+  #[inline(always)]
+  fn map_fn_to(self) -> T {
+    Self::mapping()
+  }
+}
+
+impl<F, T, Tag> MapFnTo<T, Tag> for F
+where
+  Self: UnitType,
+  T: MapFnFrom<F, Tag>,
+{
+  #[inline(always)]
+  fn mapping() -> T {
+    T::map_fn_from(F::get())
+  }
+}
+
+pub trait CFnFrom<F>
+where
+  Self: Sized,
+  F: UnitType,
+{
+  fn mapping() -> Self;
+
+  #[inline(always)]
+  fn c_fn_from(_: F) -> Self {
+    Self::mapping()
+  }
+}
+
+macro_rules! impl_c_fn_from {
+  ($($arg:ident: $ty:ident),*) => {
+    impl<F, R, $($ty),*> CFnFrom<F> for extern "C" fn($($ty),*) -> R
+    where
+      F: UnitType + Fn($($ty),*) -> R,
+    {
+      #[inline(always)]
+      fn mapping() -> Self {
+        extern "C" fn c_fn<F, R, $($ty),*>($($arg: $ty),*) -> R
+        where
+          F: UnitType + Fn($($ty),*) -> R,
+        {
+          (F::get())($($arg),*)
+        };
+        c_fn::<F, R, $($ty),*>
+      }
+    }
+  };
+}
+
+impl_c_fn_from!();
+impl_c_fn_from!(a0: A0);
+impl_c_fn_from!(a0: A0, a1: A1);
+impl_c_fn_from!(a0: A0, a1: A1, a2: A2);
+impl_c_fn_from!(a0: A0, a1: A1, a2: A2, a3: A3);
+impl_c_fn_from!(a0: A0, a1: A1, a2: A2, a3: A3, a4: A4);
+impl_c_fn_from!(a0: A0, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5);
+impl_c_fn_from!(a0: A0, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5, a6: A6);
+
+pub trait ToCFn<T>
+where
+  Self: UnitType,
+  T: Sized,
+{
+  fn mapping() -> T;
+
+  #[inline(always)]
+  fn to_c_fn(self) -> T {
+    Self::mapping()
+  }
+}
+
+impl<F, T> ToCFn<T> for F
+where
+  Self: UnitType,
+  T: CFnFrom<F>,
+{
+  #[inline(always)]
+  fn mapping() -> T {
+    T::c_fn_from(F::get())
+  }
+}

tests/test_api.rs

@@ -541,11 +541,11 @@ fn add_message_listener() {
   drop(g);
 }
 
-extern "C" fn unexpected_module_resolve_callback(
-  _context: v8::Local<v8::Context>,
-  _specifier: v8::Local<v8::String>,
-  _referrer: v8::Local<v8::Module>,
-) -> *const v8::Module {
+fn unexpected_module_resolve_callback<'a>(
+  _context: v8::Local<'a, v8::Context>,
+  _specifier: v8::Local<'a, v8::String>,
+  _referrer: v8::Local<'a, v8::Module>,
+) -> Option<v8::Local<'a, v8::Module>> {
   unreachable!()
 }
 
@@ -1289,17 +1289,17 @@ fn module_instantiation_failures1() {
     {
       let mut try_catch = v8::TryCatch::new(scope);
       let tc = try_catch.enter();
-      extern "C" fn resolve_callback(
-        context: v8::Local<v8::Context>,
-        _specifier: v8::Local<v8::String>,
-        _referrer: v8::Local<v8::Module>,
-      ) -> *const v8::Module {
+      fn resolve_callback<'a>(
+        context: v8::Local<'a, v8::Context>,
+        _specifier: v8::Local<'a, v8::String>,
+        _referrer: v8::Local<'a, v8::Module>,
+      ) -> Option<v8::Local<'a, v8::Module>> {
         let mut cbs = v8::CallbackScope::new(context);
         let mut hs = v8::HandleScope::new(cbs.enter());
         let scope = hs.enter();
         let e = v8_str(scope, "boom");
         scope.isolate().throw_exception(e.into());
-        std::ptr::null()
+        None
       }
       let result = module.instantiate_module(context, resolve_callback);
       assert!(result.is_none());
@@ -1318,11 +1318,11 @@ fn module_instantiation_failures1() {
   drop(g);
 }
 
-extern "C" fn compile_specifier_as_module_resolve_callback(
-  context: v8::Local<v8::Context>,
-  specifier: v8::Local<v8::String>,
-  _referrer: v8::Local<v8::Module>,
-) -> *const v8::Module {
+fn compile_specifier_as_module_resolve_callback<'a>(
+  context: v8::Local<'a, v8::Context>,
+  specifier: v8::Local<'a, v8::String>,
+  _referrer: v8::Local<'a, v8::Module>,
+) -> Option<v8::Local<'a, v8::Module>> {
   let mut cbs = v8::CallbackScope::new(context);
   let mut hs = v8::EscapableHandleScope::new(cbs.enter());
   let scope = hs.enter();
@@ -1330,7 +1330,7 @@ extern "C" fn compile_specifier_as_module_resolve_callback(
   let source = v8::script_compiler::Source::new(specifier, &origin);
   let module =
     v8::script_compiler::compile_module(scope.isolate(), source).unwrap();
-  &*scope.escape(module)
+  Some(scope.escape(module))
 }
 
 #[test]