Fix unsoundness in JsValueStore by replacing unsafe raw pointer mutation of Arc with safe OnceLock

core/engine/src/module/loader/mod.rs

@@ -205,15 +205,15 @@ pub trait ModuleLoader: Any {
 pub struct IdleModuleLoader;
 
 impl ModuleLoader for IdleModuleLoader {
-    async fn load_imported_module(
+    fn load_imported_module(
         self: Rc<Self>,
         _referrer: Referrer,
         _request: ModuleRequest,
         _context: &RefCell<&mut Context>,
-    ) -> JsResult<Module> {
-        Err(JsNativeError::typ()
+    ) -> impl Future<Output = JsResult<Module>> {
+        std::future::ready(Err(JsNativeError::typ()
             .with_message("module resolution is disabled for this context")
-            .into())
+            .into()))
     }
 }
 

core/engine/src/vm/mod.rs

@@ -756,7 +756,7 @@ impl Context {
         println!(
             "{:<TIME_COLUMN_WIDTH$} {:<OPCODE_COLUMN_WIDTH$} {operands:<OPERAND_COLUMN_WIDTH$} {stack}",
             format!("{}μs", duration.as_micros()),
-            format!("{}", opcode.as_str()),
+            opcode.as_str(),
             TIME_COLUMN_WIDTH = Self::TIME_COLUMN_WIDTH,
             OPCODE_COLUMN_WIDTH = Self::OPCODE_COLUMN_WIDTH,
             OPERAND_COLUMN_WIDTH = Self::OPERAND_COLUMN_WIDTH,

core/engine/tests/module.rs

@@ -11,16 +11,18 @@ use boa_engine::{Context, JsResult, JsString, Module, Source, js_string};
 fn test_json_module_from_str() {
     struct TestModuleLoader(JsString);
     impl ModuleLoader for TestModuleLoader {
-        async fn load_imported_module(
+        fn load_imported_module(
             self: Rc<Self>,
             _referrer: Referrer,
             request: boa_engine::module::ModuleRequest,
             context: &RefCell<&mut Context>,
-        ) -> JsResult<Module> {
-            assert_eq!(request.specifier().to_std_string_escaped(), "basic");
-            let src = self.0.clone();
+        ) -> impl Future<Output = JsResult<Module>> {
+            std::future::ready({
+                assert_eq!(request.specifier().to_std_string_escaped(), "basic");
+                let src = self.0.clone();
 
-            Ok(Module::parse_json(src, &mut context.borrow_mut()).unwrap())
+                Ok(Module::parse_json(src, &mut context.borrow_mut()).unwrap())
+            })
         }
     }
 
@@ -66,22 +68,24 @@ fn test_json_module_from_str() {
 fn test_json_module_dynamic_import() {
     struct TestModuleLoader(JsString);
     impl ModuleLoader for TestModuleLoader {
-        async fn load_imported_module(
+        fn load_imported_module(
             self: Rc<Self>,
             _referrer: Referrer,
             request: boa_engine::module::ModuleRequest,
             context: &RefCell<&mut Context>,
-        ) -> JsResult<Module> {
-            assert_eq!(request.specifier().to_std_string_escaped(), "basic");
-
-            // Verify attributes were passed correctly
-            let type_attr = request
-                .get_attribute("type")
-                .expect("should have type attribute");
-            assert_eq!(type_attr.to_std_string_escaped(), "json");
-
-            let src = self.0.clone();
-            Ok(Module::parse_json(src, &mut context.borrow_mut()).unwrap())
+        ) -> impl Future<Output = JsResult<Module>> {
+            std::future::ready({
+                assert_eq!(request.specifier().to_std_string_escaped(), "basic");
+
+                // Verify attributes were passed correctly
+                let type_attr = request
+                    .get_attribute("type")
+                    .expect("should have type attribute");
+                assert_eq!(type_attr.to_std_string_escaped(), "json");
+
+                let src = self.0.clone();
+                Ok(Module::parse_json(src, &mut context.borrow_mut()).unwrap())
+            })
         }
     }
 
@@ -148,21 +152,23 @@ fn test_json_module_dynamic_import() {
 fn test_json_module_static_import_with_attributes() {
     struct TestModuleLoader(JsString);
     impl ModuleLoader for TestModuleLoader {
-        async fn load_imported_module(
+        fn load_imported_module(
             self: Rc<Self>,
             _referrer: Referrer,
             request: boa_engine::module::ModuleRequest,
             context: &RefCell<&mut Context>,
-        ) -> JsResult<Module> {
-            assert_eq!(request.specifier().to_std_string_escaped(), "basic");
-
-            let type_attr = request
-                .get_attribute("type")
-                .expect("should have type attribute");
-            assert_eq!(type_attr.to_std_string_escaped(), "json");
-
-            let src = self.0.clone();
-            Ok(Module::parse_json(src, &mut context.borrow_mut()).unwrap())
+        ) -> impl Future<Output = JsResult<Module>> {
+            std::future::ready({
+                assert_eq!(request.specifier().to_std_string_escaped(), "basic");
+
+                let type_attr = request
+                    .get_attribute("type")
+                    .expect("should have type attribute");
+                assert_eq!(type_attr.to_std_string_escaped(), "json");
+
+                let src = self.0.clone();
+                Ok(Module::parse_json(src, &mut context.borrow_mut()).unwrap())
+            })
         }
     }
 
@@ -203,21 +209,23 @@ fn test_json_module_static_import_with_attributes() {
 fn test_json_module_reexport_with_attributes() {
     struct TestModuleLoader(JsString);
     impl ModuleLoader for TestModuleLoader {
-        async fn load_imported_module(
+        fn load_imported_module(
             self: Rc<Self>,
             _referrer: Referrer,
             request: boa_engine::module::ModuleRequest,
             context: &RefCell<&mut Context>,
-        ) -> JsResult<Module> {
-            assert_eq!(request.specifier().to_std_string_escaped(), "basic");
-
-            let type_attr = request
-                .get_attribute("type")
-                .expect("should have type attribute");
-            assert_eq!(type_attr.to_std_string_escaped(), "json");
-
-            let src = self.0.clone();
-            Ok(Module::parse_json(src, &mut context.borrow_mut()).unwrap())
+        ) -> impl Future<Output = JsResult<Module>> {
+            std::future::ready({
+                assert_eq!(request.specifier().to_std_string_escaped(), "basic");
+
+                let type_attr = request
+                    .get_attribute("type")
+                    .expect("should have type attribute");
+                assert_eq!(type_attr.to_std_string_escaped(), "json");
+
+                let src = self.0.clone();
+                Ok(Module::parse_json(src, &mut context.borrow_mut()).unwrap())
+            })
         }
     }
 
@@ -367,19 +375,21 @@ fn test_dynamic_import_non_string_attribute_value() {
 fn test_dynamic_import_symbol_key() {
     struct TestModuleLoader(JsString);
     impl ModuleLoader for TestModuleLoader {
-        async fn load_imported_module(
+        fn load_imported_module(
             self: Rc<Self>,
             _referrer: Referrer,
             request: boa_engine::module::ModuleRequest,
             context: &RefCell<&mut Context>,
-        ) -> JsResult<Module> {
-            assert_eq!(request.specifier().to_std_string_escaped(), "basic");
+        ) -> impl Future<Output = JsResult<Module>> {
+            std::future::ready({
+                assert_eq!(request.specifier().to_std_string_escaped(), "basic");
 
-            // Verify attributes were passed correctly (symbol key should be ignored)
-            assert!(request.get_attribute("type").is_none());
+                // Verify attributes were passed correctly (symbol key should be ignored)
+                assert!(request.get_attribute("type").is_none());
 
-            let src = self.0.clone();
-            Ok(Module::parse_json(src, &mut context.borrow_mut()).unwrap())
+                let src = self.0.clone();
+                Ok(Module::parse_json(src, &mut context.borrow_mut()).unwrap())
+            })
         }
     }
 

core/runtime/src/fetch/fetchers.rs

@@ -12,13 +12,15 @@ use std::rc::Rc;
 pub struct ErrorFetcher;
 
 impl Fetcher for ErrorFetcher {
-    async fn fetch(
+    fn fetch(
         self: Rc<Self>,
         _request: JsRequest,
         _signal: Option<JsObject>,
         _context: &RefCell<&mut Context>,
-    ) -> JsResult<JsResponse> {
-        Err(js_error!(ReferenceError: "ErrorFetcher used in fetch API."))
+    ) -> impl Future<Output = JsResult<JsResponse>> {
+        std::future::ready(Err(js_error!(
+            ReferenceError: "ErrorFetcher used in fetch API."
+        )))
     }
 }
 
@@ -32,60 +34,63 @@ pub struct BlockingReqwestFetcher {
 
 #[cfg(feature = "reqwest-blocking")]
 impl Fetcher for BlockingReqwestFetcher {
-    async fn fetch(
+    fn fetch(
         self: Rc<Self>,
         request: JsRequest,
         signal: Option<JsObject>,
         _context: &RefCell<&mut Context>,
-    ) -> JsResult<JsResponse> {
-        use boa_engine::{JsError, JsString};
+    ) -> impl Future<Output = JsResult<JsResponse>> {
+        let result = (|| {
+            use boa_engine::{JsError, JsString};
 
-        if let Some(ref sig) = signal
-            && let Some(sig_ref) = sig.downcast_ref::<crate::abort::JsAbortSignal>()
-            && sig_ref.is_aborted()
-        {
-            return Err(JsError::from_opaque(
-                boa_engine::js_string!("AbortError").into(),
-            ));
-        }
+            if let Some(ref sig) = signal
+                && let Some(sig_ref) = sig.downcast_ref::<crate::abort::JsAbortSignal>()
+                && sig_ref.is_aborted()
+            {
+                return Err(JsError::from_opaque(
+                    boa_engine::js_string!("AbortError").into(),
+                ));
+            }
 
-        let request = request.into_inner();
-        let url = request.uri().to_string();
-        let req = self
-            .client
-            .request(request.method().clone(), &url)
-            .headers(request.headers().clone());
+            let request = request.into_inner();
+            let url = request.uri().to_string();
+            let req = self
+                .client
+                .request(request.method().clone(), &url)
+                .headers(request.headers().clone());
 
-        let req = req
-            .body(request.body().clone())
-            .build()
-            .map_err(JsError::from_rust)?;
+            let req = req
+                .body(request.body().clone())
+                .build()
+                .map_err(JsError::from_rust)?;
 
-        let resp = self.client.execute(req).map_err(JsError::from_rust)?;
+            let resp = self.client.execute(req).map_err(JsError::from_rust)?;
 
-        if let Some(ref sig) = signal
-            && let Some(sig_ref) = sig.downcast_ref::<crate::abort::JsAbortSignal>()
-            && sig_ref.is_aborted()
-        {
-            return Err(JsError::from_opaque(
-                boa_engine::js_string!("AbortError").into(),
-            ));
-        }
+            if let Some(ref sig) = signal
+                && let Some(sig_ref) = sig.downcast_ref::<crate::abort::JsAbortSignal>()
+                && sig_ref.is_aborted()
+            {
+                return Err(JsError::from_opaque(
+                    boa_engine::js_string!("AbortError").into(),
+                ));
+            }
 
-        let status = resp.status();
-        let headers = resp.headers().clone();
-        let bytes = resp.bytes().map_err(JsError::from_rust)?;
-        let mut builder = http::Response::builder().status(status.as_u16());
+            let status = resp.status();
+            let headers = resp.headers().clone();
+            let bytes = resp.bytes().map_err(JsError::from_rust)?;
+            let mut builder = http::Response::builder().status(status.as_u16());
 
-        for k in headers.keys() {
-            for v in headers.get_all(k) {
-                builder = builder.header(k.as_str(), v);
+            for k in headers.keys() {
+                for v in headers.get_all(k) {
+                    builder = builder.header(k.as_str(), v);
+                }
             }
-        }
 
-        builder
-            .body(bytes.to_vec())
-            .map_err(JsError::from_rust)
-            .map(|request| JsResponse::basic(JsString::from(url), request))
+            builder
+                .body(bytes.to_vec())
+                .map_err(JsError::from_rust)
+                .map(|request| JsResponse::basic(JsString::from(url), request))
+        })();
+        async { result }
     }
 }

core/runtime/src/fetch/tests/e2e.rs

@@ -43,16 +43,18 @@ impl E2eFetcher {
 }
 
 impl crate::fetch::Fetcher for E2eFetcher {
-    async fn fetch(
+    fn fetch(
         self: Rc<Self>,
         request: JsRequest,
         _signal: Option<boa_engine::JsObject>,
         context: &RefCell<&mut Context>,
-    ) -> JsResult<JsResponse> {
-        match request.uri().path() {
-            "/headers" => Self::headers(&request, &mut context.borrow_mut()),
-            _ => Err(js_error!("Invalid request.")),
-        }
+    ) -> impl Future<Output = JsResult<JsResponse>> {
+        std::future::ready({
+            match request.uri().path() {
+                "/headers" => Self::headers(&request, &mut context.borrow_mut()),
+                _ => Err(js_error!("Invalid request.")),
+            }
+        })
     }
 }
 

core/runtime/src/fetch/tests/mod.rs

@@ -38,19 +38,21 @@ impl TestFetcher {
 }
 
 impl crate::fetch::Fetcher for TestFetcher {
-    async fn fetch(
+    fn fetch(
         self: Rc<Self>,
         request: JsRequest,
         _signal: Option<boa_engine::JsObject>,
         _context: &RefCell<&mut Context>,
-    ) -> JsResult<JsResponse> {
-        let request = request.into_inner();
-        self.requests_received.borrow_mut().push(request.clone());
-        let url = request.uri();
-        self.request_mapper
-            .get(url)
-            .cloned()
-            .map(|response| JsResponse::basic(JsString::from(url.to_string()), response))
-            .ok_or_else(|| js_error!("No response found for URL"))
+    ) -> impl Future<Output = JsResult<JsResponse>> {
+        std::future::ready({
+            let request = request.into_inner();
+            self.requests_received.borrow_mut().push(request.clone());
+            let url = request.uri();
+            self.request_mapper
+                .get(url)
+                .cloned()
+                .map(|response| JsResponse::basic(JsString::from(url.to_string()), response))
+                .ok_or_else(|| js_error!("No response found for URL"))
+        })
     }
 }

core/runtime/src/fetch/tests/response.rs

@@ -108,10 +108,7 @@ fn response_json() {
         TestAction::inspect_context(|ctx| {
             let response = ctx.global_object().get(js_str!("response"), ctx).unwrap();
             let response = response.as_promise().unwrap().await_blocking(ctx).unwrap();
-            assert_eq!(
-                format!("{}", response.display_obj(false)),
-                "{\n    hello world: 123\n}"
-            );
+            assert_eq!(response.display_obj(false), "{\n    hello world: 123\n}");
         }),
     ]);
 }