Expose dynamic import in core (#2472)

Author: ry

core/isolate.rs

@@ -7,6 +7,7 @@
 use crate::js_errors::JSError;
 use crate::libdeno;
 use crate::libdeno::deno_buf;
+use crate::libdeno::deno_dyn_import_id;
 use crate::libdeno::deno_mod;
 use crate::libdeno::deno_pinned_buf;
 use crate::libdeno::PinnedBuf;
@@ -19,6 +20,7 @@ use futures::task;
 use futures::Async::*;
 use futures::Future;
 use futures::Poll;
+use libc::c_char;
 use libc::c_void;
 use std::ffi::CStr;
 use std::ffi::CString;
@@ -52,9 +54,33 @@ pub enum StartupData<'a> {
 
 type DispatchFn = Fn(&[u8], Option<PinnedBuf>) -> Op;
 
+pub type DynImportFuture = Box<dyn Future<Item = deno_mod, Error = ()> + Send>;
+type DynImportFn = Fn(&str, &str) -> DynImportFuture;
+
+/// Wraps DynImportFuture to include the deno_dyn_import_id, so that it doesn't
+/// need to be exposed.
+struct DynImport {
+  id: deno_dyn_import_id,
+  inner: DynImportFuture,
+}
+
+impl Future for DynImport {
+  type Item = (deno_dyn_import_id, deno_mod);
+  type Error = ();
+  fn poll(&mut self) -> Poll<Self::Item, ()> {
+    match self.inner.poll() {
+      Ok(Ready(mod_id)) => Ok(Ready((self.id, mod_id))),
+      Ok(NotReady) => Ok(NotReady),
+      // Note that mod_id 0 indicates error.
+      Err(()) => Ok(Ready((self.id, 0))),
+    }
+  }
+}
+
 #[derive(Default)]
 pub struct Config {
   dispatch: Option<Arc<DispatchFn>>,
+  dyn_import: Option<Arc<DynImportFn>>,
   pub will_snapshot: bool,
 }
 
@@ -68,6 +94,13 @@ impl Config {
   {
     self.dispatch = Some(Arc::new(f));
   }
+
+  pub fn dyn_import<F>(&mut self, f: F)
+  where
+    F: Fn(&str, &str) -> DynImportFuture + Send + Sync + 'static,
+  {
+    self.dyn_import = Some(Arc::new(f));
+  }
 }
 
 /// A single execution context of JavaScript. Corresponds roughly to the "Web
@@ -85,6 +118,7 @@ pub struct Isolate {
   needs_init: bool,
   shared: SharedQueue,
   pending_ops: FuturesUnordered<OpAsyncFuture>,
+  pending_dyn_imports: FuturesUnordered<DynImport>,
   have_unpolled_ops: bool,
 }
 
@@ -121,6 +155,7 @@ impl Isolate {
       load_snapshot: Snapshot2::empty(),
       shared: shared.as_deno_buf(),
       recv_cb: Self::pre_dispatch,
+      dyn_import_cb: Self::dyn_import,
     };
 
     // Seperate into Option values for each startup type
@@ -147,6 +182,7 @@ impl Isolate {
       needs_init,
       pending_ops: FuturesUnordered::new(),
       have_unpolled_ops: false,
+      pending_dyn_imports: FuturesUnordered::new(),
     };
 
     // If we want to use execute this has to happen here sadly.
@@ -174,6 +210,28 @@ impl Isolate {
     }
   }
 
+  extern "C" fn dyn_import(
+    user_data: *mut c_void,
+    specifier: *const c_char,
+    referrer: *const c_char,
+    id: deno_dyn_import_id,
+  ) {
+    assert_ne!(user_data, std::ptr::null_mut());
+    let isolate = unsafe { Isolate::from_raw_ptr(user_data) };
+    let specifier = unsafe { CStr::from_ptr(specifier).to_str().unwrap() };
+    let referrer = unsafe { CStr::from_ptr(referrer).to_str().unwrap() };
+    debug!("dyn_import specifier {} referrer {} ", specifier, referrer);
+
+    if let Some(ref f) = isolate.config.dyn_import {
+      let inner = f(specifier, referrer);
+      let fut = DynImport { inner, id };
+      task::current().notify();
+      isolate.pending_dyn_imports.push(fut);
+    } else {
+      panic!("dyn_import callback not set")
+    }
+  }
+
   extern "C" fn pre_dispatch(
     user_data: *mut c_void,
     control_argv0: deno_buf,
@@ -340,6 +398,27 @@ impl Isolate {
     assert_ne!(snapshot.data_len, 0);
     Ok(snapshot)
   }
+
+  fn dyn_import_done(
+    &self,
+    id: libdeno::deno_dyn_import_id,
+    mod_id: deno_mod,
+  ) -> Result<(), JSError> {
+    debug!("dyn_import_done {} {}", id, mod_id);
+    unsafe {
+      libdeno::deno_dyn_import(
+        self.libdeno_isolate,
+        self.as_raw_ptr(),
+        id,
+        mod_id,
+      )
+    };
+    if let Some(js_error) = self.last_exception() {
+      assert_eq!(id, 0);
+      return Err(js_error);
+    }
+    Ok(())
+  }
 }
 
 /// Called during mod_instantiate() to resolve imports.
@@ -440,6 +519,18 @@ impl Future for Isolate {
     let mut overflow_response: Option<Buf> = None;
 
     loop {
+      // If there are any pending dyn_import futures, do those first.
+      match self.pending_dyn_imports.poll() {
+        Err(()) => unreachable!(),
+        Ok(NotReady) => unreachable!(),
+        Ok(Ready(None)) => (),
+        Ok(Ready(Some((dyn_import_id, mod_id)))) => {
+          self.dyn_import_done(dyn_import_id, mod_id)?;
+          continue;
+        }
+      }
+
+      // Now handle actual ops.
       self.have_unpolled_ops = false;
       #[allow(clippy::match_wild_err_arm)]
       match self.pending_ops.poll() {
@@ -764,6 +855,95 @@ pub mod tests {
     });
   }
 
+  #[test]
+  fn dyn_import_err() {
+    // Test an erroneous dynamic import where the specified module isn't found.
+    run_in_task(|| {
+      let count = Arc::new(AtomicUsize::new(0));
+      let count_ = count.clone();
+      let mut config = Config::default();
+      config.dyn_import(move |specifier, referrer| {
+        count_.fetch_add(1, Ordering::Relaxed);
+        assert_eq!(specifier, "foo.js");
+        assert_eq!(referrer, "dyn_import2.js");
+        Box::new(futures::future::err(()))
+      });
+      let mut isolate = Isolate::new(StartupData::None, config);
+      js_check(isolate.execute(
+        "dyn_import2.js",
+        r#"
+        (async () => {
+          await import("foo.js");
+        })();
+        "#,
+      ));
+      assert_eq!(count.load(Ordering::Relaxed), 1);
+
+      // We should get an error here.
+      let result = isolate.poll();
+      assert!(result.is_err());
+    })
+  }
+
+  #[test]
+  fn dyn_import_ok() {
+    run_in_task(|| {
+      let count = Arc::new(AtomicUsize::new(0));
+      let count_ = count.clone();
+
+      // Sometimes Rust is really annoying.
+      use std::sync::Mutex;
+      let mod_b = Arc::new(Mutex::new(0));
+      let mod_b2 = mod_b.clone();
+
+      let mut config = Config::default();
+      config.dyn_import(move |_specifier, referrer| {
+        count_.fetch_add(1, Ordering::Relaxed);
+        // assert_eq!(specifier, "foo.js");
+        assert_eq!(referrer, "dyn_import3.js");
+        let mod_id = mod_b2.lock().unwrap();
+        Box::new(futures::future::ok(*mod_id))
+      });
+
+      let mut isolate = Isolate::new(StartupData::None, config);
+
+      // Instantiate mod_b
+      {
+        let mut mod_id = mod_b.lock().unwrap();
+        *mod_id = isolate
+          .mod_new(false, "b.js", "export function b() { return 'b' }")
+          .unwrap();
+        let mut resolve = move |_specifier: &str,
+                                _referrer: deno_mod|
+              -> deno_mod { unreachable!() };
+        js_check(isolate.mod_instantiate(*mod_id, &mut resolve));
+      }
+      // Dynamically import mod_b
+      js_check(isolate.execute(
+        "dyn_import3.js",
+        r#"
+        (async () => {
+          let mod = await import("foo1.js");
+          if (mod.b() !== 'b') {
+            throw Error("bad1");
+          }
+          // And again!
+          mod = await import("foo2.js");
+          if (mod.b() !== 'b') {
+            throw Error("bad2");
+          }
+        })();
+        "#,
+      ));
+
+      assert_eq!(count.load(Ordering::Relaxed), 1);
+      assert_eq!(Ok(Ready(())), isolate.poll());
+      assert_eq!(count.load(Ordering::Relaxed), 2);
+      assert_eq!(Ok(Ready(())), isolate.poll());
+      assert_eq!(count.load(Ordering::Relaxed), 2);
+    })
+  }
+
   #[test]
   fn terminate_execution() {
     let (tx, rx) = std::sync::mpsc::channel::<bool>();

core/libdeno.rs

@@ -192,9 +192,23 @@ type deno_recv_cb = unsafe extern "C" fn(
   zero_copy_buf: deno_pinned_buf,
 );
 
+/// Called when dynamic import is called in JS: import('foo')
+/// Embedder must call deno_dyn_import() with the specified id and
+/// the module.
+#[allow(non_camel_case_types)]
+type deno_dyn_import_cb = unsafe extern "C" fn(
+  user_data: *mut c_void,
+  specifier: *const c_char,
+  referrer: *const c_char,
+  id: deno_dyn_import_id,
+);
+
 #[allow(non_camel_case_types)]
 pub type deno_mod = i32;
 
+#[allow(non_camel_case_types)]
+pub type deno_dyn_import_id = i32;
+
 #[allow(non_camel_case_types)]
 type deno_resolve_cb = unsafe extern "C" fn(
   user_data: *mut c_void,
@@ -208,6 +222,7 @@ pub struct deno_config<'a> {
   pub load_snapshot: Snapshot2<'a>,
   pub shared: deno_buf,
   pub recv_cb: deno_recv_cb,
+  pub dyn_import_cb: deno_dyn_import_cb,
 }
 
 #[cfg(not(windows))]
@@ -292,6 +307,14 @@ extern "C" {
     id: deno_mod,
   );
 
+  /// Call exactly once for every deno_dyn_import_cb.
+  pub fn deno_dyn_import(
+    i: *const isolate,
+    user_data: *const c_void,
+    id: deno_dyn_import_id,
+    mod_id: deno_mod,
+  );
+
   pub fn deno_snapshot_new(i: *const isolate) -> Snapshot1<'static>;
 
   #[allow(dead_code)]

core/libdeno/binding.cc

@@ -521,6 +521,8 @@ v8::MaybeLocal<v8::Promise> HostImportModuleDynamicallyCallback(
   auto* isolate = context->GetIsolate();
   DenoIsolate* d = DenoIsolate::FromIsolate(isolate);
   v8::Isolate::Scope isolate_scope(isolate);
+  v8::Context::Scope context_scope(context);
+  v8::EscapableHandleScope handle_scope(isolate);
 
   v8::String::Utf8Value specifier_str(isolate, specifier);
 
@@ -544,7 +546,9 @@ v8::MaybeLocal<v8::Promise> HostImportModuleDynamicallyCallback(
 
   d->dyn_import_cb_(d->user_data_, *specifier_str, *referrer_name_str,
                     import_id);
-  return resolver->GetPromise();
+
+  auto promise = resolver->GetPromise();
+  return handle_scope.Escape(promise);
 }
 
 void DenoIsolate::AddIsolate(v8::Isolate* isolate) {

core/libdeno/deno.h

@@ -126,7 +126,9 @@ void deno_mod_instantiate(Deno* d, void* user_data, deno_mod id,
 void deno_mod_evaluate(Deno* d, void* user_data, deno_mod id);
 
 // Call exactly once for every deno_dyn_import_cb.
-void deno_dyn_import(Deno* d, deno_dyn_import_id id, deno_mod mod_id);
+// Note this call will execute JS.
+void deno_dyn_import(Deno* d, void* user_data, deno_dyn_import_id id,
+                     deno_mod mod_id);
 
 #ifdef __cplusplus
 }  // extern "C"

core/libdeno/modules.cc

@@ -151,13 +151,19 @@ void deno_mod_evaluate(Deno* d_, void* user_data, deno_mod id) {
   }
 }
 
-void deno_dyn_import(Deno* d_, deno_dyn_import_id import_id, deno_mod mod_id) {
+void deno_dyn_import(Deno* d_, void* user_data, deno_dyn_import_id import_id,
+                     deno_mod mod_id) {
   auto* d = unwrap(d_);
+  deno::UserDataScope user_data_scope(d, user_data);
+
   auto* isolate = d->isolate_;
   v8::Isolate::Scope isolate_scope(isolate);
   v8::Locker locker(isolate);
   v8::HandleScope handle_scope(isolate);
   auto context = d->context_.Get(d->isolate_);
+  v8::Context::Scope context_scope(context);
+
+  v8::TryCatch try_catch(isolate);
 
   auto it = d->dyn_import_map_.find(import_id);
   if (it == d->dyn_import_map_.end()) {
@@ -168,9 +174,13 @@ void deno_dyn_import(Deno* d_, deno_dyn_import_id import_id, deno_mod mod_id) {
   /// Resolve.
   auto persistent_promise = &it->second;
   auto promise = persistent_promise->Get(isolate);
-  persistent_promise->Reset();
 
   auto* info = d->GetModuleInfo(mod_id);
+
+  // Do the following callback into JS?? Is user_data_scope needed?
+  persistent_promise->Reset();
+  d->dyn_import_map_.erase(it);
+
   if (info == nullptr) {
     // Resolution error.
     promise->Reject(context, v8::Null(isolate)).ToChecked();
@@ -181,7 +191,10 @@ void deno_dyn_import(Deno* d_, deno_dyn_import_id import_id, deno_mod mod_id) {
     Local<Value> module_namespace = module->GetModuleNamespace();
     promise->Resolve(context, module_namespace).ToChecked();
   }
-  d->dyn_import_map_.erase(it);
+
+  if (try_catch.HasCaught()) {
+    HandleException(context, try_catch.Exception());
+  }
 }
 
 }  // extern "C"