Don't free resolved modules and add tests

core/src/context/ctx.rs

@@ -407,6 +407,19 @@ impl<'js> Ctx<'js> {
     pub fn as_raw(&self) -> NonNull<qjs::JSContext> {
         self.ctx
     }
+
+    /// Frees modules which aren't eveluated.
+    ///
+    /// When a module is compiled and the compilation results in an error the module can already
+    /// have resolved several modules. Originally quickjs freed all these module when compiling
+    /// (but not when a it was dynamically imported), this library patched that behaviour out
+    /// because it proved to be hard to make safe. This function will free those modules.
+    ///
+    /// # Safety
+    /// Caller must ensure that this method is not called from a module being evaluated.
+    pub unsafe fn free_unevaluated_modules(&self) {
+        qjs::JS_FreeUnevaluatedModules(self.ctx.as_ptr())
+    }
 }
 
 #[cfg(test)]

core/src/value/module.rs

@@ -906,6 +906,7 @@ where
 
 #[cfg(test)]
 mod test {
+
     use super::*;
     use crate::*;
 
@@ -999,6 +1000,42 @@ mod test {
         });
     }
 
+    #[test]
+    fn holding_onto_unevaluated() {
+        let runtime = Runtime::new().unwrap();
+        let ctx = Context::full(&runtime).unwrap();
+        ctx.with(|ctx| {
+            let module = unsafe {
+                Module::unsafe_declare(
+                    ctx.clone(),
+                    "test",
+                    "export function add(a,b){ return a + b }",
+                )
+                .unwrap()
+            };
+            // Error
+            ctx.compile("test2", "throw new Error(1)").ok();
+
+            unsafe { module.eval().unwrap() }
+        });
+    }
+
+    #[test]
+    fn eval_crashing_module_inside_module() {
+        let runtime = Runtime::new().unwrap();
+        let ctx = Context::full(&runtime).unwrap();
+
+        ctx.with(|ctx| {
+            let globals = ctx.globals();
+            let eval_crashing = |ctx: Ctx| ctx.compile("test2", "throw new Error(1)").map(|_| ());
+            let function = Function::new(ctx.clone(), eval_crashing).unwrap();
+            globals.set("eval_crashing", function).unwrap();
+
+            let res = ctx.compile("test", " eval_crashing(); ");
+            assert!(res.is_err())
+        });
+    }
+
     #[test]
     fn from_javascript() {
         test_with(|ctx| {

sys/patches/dynamic_import_sync.patch

@@ -1,5 +1,5 @@
 diff --git a/quickjs.c b/quickjs.c
-index 48aeffc..9e8c97f 100644
+index 48aeffc..6862b8c 100644
 --- a/quickjs.c
 +++ b/quickjs.c
 @@ -1192,6 +1192,8 @@ static void js_free_module_def(JSContext *ctx, JSModuleDef *m);
@@ -136,11 +136,41 @@ index 48aeffc..9e8c97f 100644
  static __exception JSAtom js_parse_from_clause(JSParseState *s)
  {
      JSAtom module_name;
+@@ -33532,7 +33584,7 @@ static JSValue JS_EvalFunctionInternal(JSContext *ctx, JSValue fun_obj,
+         ret_val = js_evaluate_module(ctx, m);
+         if (JS_IsException(ret_val)) {
+         fail:
+-            js_free_modules(ctx, JS_FREE_MODULE_NOT_EVALUATED);
++            js_free_modules(ctx, JS_FREE_MODULE_NOT_RESOLVED);
+             return JS_EXCEPTION;
+         }
+     } else {
+@@ -33542,6 +33594,10 @@ static JSValue JS_EvalFunctionInternal(JSContext *ctx, JSValue fun_obj,
+     return ret_val;
+ }
+
++void JS_FreeUnevaluatedModules(JSContext *ctx){
++    js_free_modules(ctx, JS_FREE_MODULE_NOT_EVALUATED);
++}
++
+ JSValue JS_EvalFunction(JSContext *ctx, JSValue fun_obj)
+ {
+     return JS_EvalFunctionInternal(ctx, fun_obj, ctx->global_obj, NULL, NULL);
 diff --git a/quickjs.h b/quickjs.h
-index d4a5cd3..991a663 100644
+index d4a5cd3..0ad5b30 100644
 --- a/quickjs.h
 +++ b/quickjs.h
-@@ -1039,6 +1039,8 @@ int JS_SetModuleExport(JSContext *ctx, JSModuleDef *m, const char *export_name,
+@@ -866,6 +866,9 @@ void JS_SetModuleLoaderFunc(JSRuntime *rt,
+ JSValue JS_GetImportMeta(JSContext *ctx, JSModuleDef *m);
+ JSAtom JS_GetModuleName(JSContext *ctx, JSModuleDef *m);
+
++void JS_FreeUnevaluatedModules(JSContext *ctx);
++
++
+ /* JS Job support */
+
+ typedef JSValue JSJobFunc(JSContext *ctx, int argc, JSValueConst *argv);
+@@ -1039,6 +1042,8 @@ int JS_SetModuleExport(JSContext *ctx, JSModuleDef *m, const char *export_name,
  int JS_SetModuleExportList(JSContext *ctx, JSModuleDef *m,
                             const JSCFunctionListEntry *tab, int len);
 

sys/src/bindings/aarch64-apple-darwin.rs

@@ -1882,6 +1882,9 @@ extern "C" {
 extern "C" {
     pub fn JS_GetModuleName(ctx: *mut JSContext, m: *mut JSModuleDef) -> JSAtom;
 }
+extern "C" {
+    pub fn JS_FreeUnevaluatedModules(ctx: *mut JSContext);
+}
 pub type JSJobFunc = ::std::option::Option<
     unsafe extern "C" fn(
         ctx: *mut JSContext,

sys/src/bindings/aarch64-unknown-linux-musl.rs

@@ -1881,6 +1881,9 @@ extern "C" {
 extern "C" {
     pub fn JS_GetModuleName(ctx: *mut JSContext, m: *mut JSModuleDef) -> JSAtom;
 }
+extern "C" {
+    pub fn JS_FreeUnevaluatedModules(ctx: *mut JSContext);
+}
 pub type JSJobFunc = ::std::option::Option<
     unsafe extern "C" fn(
         ctx: *mut JSContext,

sys/src/bindings/i686-pc-windows-gnu.rs

@@ -1789,6 +1789,9 @@ extern "C" {
 extern "C" {
     pub fn JS_GetModuleName(ctx: *mut JSContext, m: *mut JSModuleDef) -> JSAtom;
 }
+extern "C" {
+    pub fn JS_FreeUnevaluatedModules(ctx: *mut JSContext);
+}
 pub type JSJobFunc = ::std::option::Option<
     unsafe extern "C" fn(
         ctx: *mut JSContext,

sys/src/bindings/i686-pc-windows-msvc.rs

@@ -1789,6 +1789,9 @@ extern "C" {
 extern "C" {
     pub fn JS_GetModuleName(ctx: *mut JSContext, m: *mut JSModuleDef) -> JSAtom;
 }
+extern "C" {
+    pub fn JS_FreeUnevaluatedModules(ctx: *mut JSContext);
+}
 pub type JSJobFunc = ::std::option::Option<
     unsafe extern "C" fn(
         ctx: *mut JSContext,

sys/src/bindings/i686-unknown-linux-gnu.rs

@@ -1789,6 +1789,9 @@ extern "C" {
 extern "C" {
     pub fn JS_GetModuleName(ctx: *mut JSContext, m: *mut JSModuleDef) -> JSAtom;
 }
+extern "C" {
+    pub fn JS_FreeUnevaluatedModules(ctx: *mut JSContext);
+}
 pub type JSJobFunc = ::std::option::Option<
     unsafe extern "C" fn(
         ctx: *mut JSContext,

sys/src/bindings/x86_64-apple-darwin.rs

@@ -1882,6 +1882,9 @@ extern "C" {
 extern "C" {
     pub fn JS_GetModuleName(ctx: *mut JSContext, m: *mut JSModuleDef) -> JSAtom;
 }
+extern "C" {
+    pub fn JS_FreeUnevaluatedModules(ctx: *mut JSContext);
+}
 pub type JSJobFunc = ::std::option::Option<
     unsafe extern "C" fn(
         ctx: *mut JSContext,

sys/src/bindings/x86_64-pc-windows-gnu.rs

@@ -1881,6 +1881,9 @@ extern "C" {
 extern "C" {
     pub fn JS_GetModuleName(ctx: *mut JSContext, m: *mut JSModuleDef) -> JSAtom;
 }
+extern "C" {
+    pub fn JS_FreeUnevaluatedModules(ctx: *mut JSContext);
+}
 pub type JSJobFunc = ::std::option::Option<
     unsafe extern "C" fn(
         ctx: *mut JSContext,

sys/src/bindings/x86_64-pc-windows-msvc.rs

@@ -1881,6 +1881,9 @@ extern "C" {
 extern "C" {
     pub fn JS_GetModuleName(ctx: *mut JSContext, m: *mut JSModuleDef) -> JSAtom;
 }
+extern "C" {
+    pub fn JS_FreeUnevaluatedModules(ctx: *mut JSContext);
+}
 pub type JSJobFunc = ::std::option::Option<
     unsafe extern "C" fn(
         ctx: *mut JSContext,

sys/src/bindings/x86_64-unknown-linux-gnu.rs

@@ -1881,6 +1881,9 @@ extern "C" {
 extern "C" {
     pub fn JS_GetModuleName(ctx: *mut JSContext, m: *mut JSModuleDef) -> JSAtom;
 }
+extern "C" {
+    pub fn JS_FreeUnevaluatedModules(ctx: *mut JSContext);
+}
 pub type JSJobFunc = ::std::option::Option<
     unsafe extern "C" fn(
         ctx: *mut JSContext,

sys/src/bindings/x86_64-unknown-linux-musl.rs

@@ -1881,6 +1881,9 @@ extern "C" {
 extern "C" {
     pub fn JS_GetModuleName(ctx: *mut JSContext, m: *mut JSModuleDef) -> JSAtom;
 }
+extern "C" {
+    pub fn JS_FreeUnevaluatedModules(ctx: *mut JSContext);
+}
 pub type JSJobFunc = ::std::option::Option<
     unsafe extern "C" fn(
         ctx: *mut JSContext,