use ThreadSafeJSContext instead of ContextForRequestInterrupt (#33769)

* use `ThreadSafeJSContext` instead of `ContextForRequestInterrupt`

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>

* use servo/mozjs

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>

---------

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>

Cargo.lock

@@ -4519,7 +4519,7 @@ dependencies = [
 [[package]]
 name = "mozjs"
 version = "0.14.1"
-source = "git+https://github.com/servo/mozjs#446ca9765ac3fe582147fd57a51d9a3bcea676ec"
+source = "git+https://github.com/servo/mozjs#a02aaf1e11fd275f2f129d0c7ca80a9d07460036"
 dependencies = [
  "bindgen",
  "cc",
@@ -4531,8 +4531,8 @@ dependencies = [
 
 [[package]]
 name = "mozjs_sys"
-version = "0.128.0-12"
-source = "git+https://github.com/servo/mozjs#446ca9765ac3fe582147fd57a51d9a3bcea676ec"
+version = "0.128.3-0"
+source = "git+https://github.com/servo/mozjs#a02aaf1e11fd275f2f129d0c7ca80a9d07460036"
 dependencies = [
  "bindgen",
  "cc",

components/script/dom/bindings/trace.rs

@@ -64,7 +64,7 @@ use crate::dom::bindings::str::{DOMString, USVString};
 use crate::dom::htmlimageelement::SourceSet;
 use crate::dom::htmlmediaelement::HTMLMediaElementFetchContext;
 use crate::dom::windowproxy::WindowProxyHandler;
-use crate::script_runtime::{ContextForRequestInterrupt, StreamConsumer};
+use crate::script_runtime::StreamConsumer;
 use crate::script_thread::IncompleteParserContexts;
 use crate::task::TaskBox;
 
@@ -359,7 +359,6 @@ where
 unsafe_no_jsmanaged_fields!(Error);
 unsafe_no_jsmanaged_fields!(TrustedPromise);
 
-unsafe_no_jsmanaged_fields!(ContextForRequestInterrupt);
 unsafe_no_jsmanaged_fields!(WindowProxyHandler);
 unsafe_no_jsmanaged_fields!(DOMString);
 unsafe_no_jsmanaged_fields!(USVString);

components/script/dom/dedicatedworkerglobalscope.rs

@@ -55,8 +55,8 @@ use crate::fetch::load_whole_resource;
 use crate::realms::{enter_realm, AlreadyInRealm, InRealm};
 use crate::script_runtime::ScriptThreadEventCategory::WorkerEvent;
 use crate::script_runtime::{
-    new_child_runtime, CanGc, CommonScriptMsg, ContextForRequestInterrupt,
-    JSContext as SafeJSContext, Runtime, ScriptChan, ScriptPort,
+    new_child_runtime, CanGc, CommonScriptMsg, JSContext as SafeJSContext, Runtime, ScriptChan,
+    ScriptPort, ThreadSafeJSContext,
 };
 use crate::task_queue::{QueuedTask, QueuedTaskConversion, TaskQueue};
 use crate::task_source::networking::NetworkingTaskSource;
@@ -332,7 +332,7 @@ impl DedicatedWorkerGlobalScope {
         browsing_context: Option<BrowsingContextId>,
         gpu_id_hub: Arc<IdentityHub>,
         control_receiver: Receiver<DedicatedWorkerControlMsg>,
-        context_sender: Sender<ContextForRequestInterrupt>,
+        context_sender: Sender<ThreadSafeJSContext>,
         can_gc: CanGc,
     ) -> JoinHandle<()> {
         let serialized_worker_url = worker_url.to_string();
@@ -384,8 +384,8 @@ impl DedicatedWorkerGlobalScope {
                     new_child_runtime(parent, Some(task_source))
                 };
 
-                let context_for_interrupt = ContextForRequestInterrupt::new(runtime.cx());
-                let _ = context_sender.send(context_for_interrupt.clone());
+                let context_for_interrupt = runtime.thread_safe_js_context();
+                let _ = context_sender.send(context_for_interrupt);
 
                 let (devtools_mpsc_chan, devtools_mpsc_port) = unbounded();
                 ROUTER.route_ipc_receiver_to_crossbeam_sender(
@@ -442,7 +442,7 @@ impl DedicatedWorkerGlobalScope {
                                 TaskSourceName::DOMManipulation,
                             ))
                             .unwrap();
-                        scope.clear_js_runtime(context_for_interrupt);
+                        scope.clear_js_runtime();
                         return;
                     },
                     Ok((metadata, bytes)) => (metadata, bytes),
@@ -457,7 +457,7 @@ impl DedicatedWorkerGlobalScope {
                 }
 
                 if scope.is_closing() {
-                    scope.clear_js_runtime(context_for_interrupt);
+                    scope.clear_js_runtime();
                     return;
                 }
 
@@ -487,7 +487,7 @@ impl DedicatedWorkerGlobalScope {
                         CommonScriptMsg::CollectReports,
                     );
 
-                scope.clear_js_runtime(context_for_interrupt);
+                scope.clear_js_runtime();
             })
             .expect("Thread spawning failed")
     }

components/script/dom/globalscope.rs

@@ -120,8 +120,7 @@ use crate::microtask::{Microtask, MicrotaskQueue, UserMicrotask};
 use crate::realms::{enter_realm, AlreadyInRealm, InRealm};
 use crate::script_module::{DynamicModuleList, ModuleScript, ModuleTree, ScriptFetchOptions};
 use crate::script_runtime::{
-    CanGc, CommonScriptMsg, ContextForRequestInterrupt, JSContext as SafeJSContext, ScriptChan,
-    ScriptPort,
+    CanGc, CommonScriptMsg, JSContext as SafeJSContext, ScriptChan, ScriptPort, ThreadSafeJSContext,
 };
 use crate::script_thread::{MainThreadScriptChan, ScriptThread};
 use crate::security_manager::CSPViolationReporter;
@@ -151,7 +150,8 @@ pub struct AutoCloseWorker {
     #[no_trace]
     control_sender: Sender<DedicatedWorkerControlMsg>,
     /// The context to request an interrupt on the worker thread.
-    context: ContextForRequestInterrupt,
+    #[no_trace]
+    context: ThreadSafeJSContext,
 }
 
 impl Drop for AutoCloseWorker {
@@ -168,7 +168,7 @@ impl Drop for AutoCloseWorker {
             warn!("Couldn't send an exit message to a dedicated worker.");
         }
 
-        self.context.request_interrupt();
+        self.context.request_interrupt_callback();
 
         // TODO: step 2 and 3.
         // Step 4 is unnecessary since we don't use actual ports for dedicated workers.
@@ -2115,7 +2115,7 @@ impl GlobalScope {
         closing: Arc<AtomicBool>,
         join_handle: JoinHandle<()>,
         control_sender: Sender<DedicatedWorkerControlMsg>,
-        context: ContextForRequestInterrupt,
+        context: ThreadSafeJSContext,
     ) {
         self.list_auto_close_worker
             .borrow_mut()

components/script/dom/serviceworkerglobalscope.rs

@@ -45,8 +45,8 @@ use crate::dom::workerglobalscope::WorkerGlobalScope;
 use crate::fetch::load_whole_resource;
 use crate::realms::{enter_realm, AlreadyInRealm, InRealm};
 use crate::script_runtime::{
-    new_rt_and_cx, CanGc, CommonScriptMsg, ContextForRequestInterrupt, JSContext as SafeJSContext,
-    Runtime, ScriptChan,
+    new_rt_and_cx, CanGc, CommonScriptMsg, JSContext as SafeJSContext, Runtime, ScriptChan,
+    ThreadSafeJSContext,
 };
 use crate::task_queue::{QueuedTask, QueuedTaskConversion, TaskQueue};
 use crate::task_source::TaskSourceName;
@@ -293,7 +293,7 @@ impl ServiceWorkerGlobalScope {
         swmanager_sender: IpcSender<ServiceWorkerMsg>,
         scope_url: ServoUrl,
         control_receiver: Receiver<ServiceWorkerControlMsg>,
-        context_sender: Sender<ContextForRequestInterrupt>,
+        context_sender: Sender<ThreadSafeJSContext>,
         closing: Arc<AtomicBool>,
         can_gc: CanGc,
     ) -> JoinHandle<()> {
@@ -311,8 +311,8 @@ impl ServiceWorkerGlobalScope {
             .spawn(move || {
                 thread_state::initialize(ThreadState::SCRIPT | ThreadState::IN_WORKER);
                 let runtime = new_rt_and_cx(None);
-                let context_for_interrupt = ContextForRequestInterrupt::new(runtime.cx());
-                let _ = context_sender.send(context_for_interrupt.clone());
+                let context_for_interrupt = runtime.thread_safe_js_context();
+                let _ = context_sender.send(context_for_interrupt);
 
                 let roots = RootCollection::new();
                 let _stack_roots = ThreadLocalStackRoots::new(&roots);
@@ -366,7 +366,7 @@ impl ServiceWorkerGlobalScope {
                     match load_whole_resource(request, &resource_threads_sender, global.upcast()) {
                         Err(_) => {
                             println!("error loading script {}", serialized_worker_url);
-                            scope.clear_js_runtime(context_for_interrupt);
+                            scope.clear_js_runtime();
                             return;
                         },
                         Ok((metadata, bytes)) => {
@@ -407,7 +407,7 @@ impl ServiceWorkerGlobalScope {
                         CommonScriptMsg::CollectReports,
                     );
 
-                scope.clear_js_runtime(context_for_interrupt);
+                scope.clear_js_runtime();
             })
             .expect("Thread spawning failed")
     }

components/script/dom/worker.rs

@@ -37,7 +37,7 @@ use crate::dom::messageevent::MessageEvent;
 use crate::dom::window::Window;
 use crate::dom::workerglobalscope::prepare_workerscope_init;
 use crate::realms::enter_realm;
-use crate::script_runtime::{CanGc, ContextForRequestInterrupt, JSContext};
+use crate::script_runtime::{CanGc, JSContext, ThreadSafeJSContext};
 use crate::task::TaskOnce;
 
 pub type TrustedWorkerAddress = Trusted<Worker>;
@@ -55,7 +55,8 @@ pub struct Worker {
     closing: Arc<AtomicBool>,
     terminated: Cell<bool>,
     #[ignore_malloc_size_of = "Arc"]
-    context_for_interrupt: DomRefCell<Option<ContextForRequestInterrupt>>,
+    #[no_trace]
+    context_for_interrupt: DomRefCell<Option<ThreadSafeJSContext>>,
 }
 
 impl Worker {
@@ -88,7 +89,7 @@ impl Worker {
         self.terminated.get()
     }
 
-    pub fn set_context_for_interrupt(&self, cx: ContextForRequestInterrupt) {
+    pub fn set_context_for_interrupt(&self, cx: ThreadSafeJSContext) {
         assert!(
             self.context_for_interrupt.borrow().is_none(),
             "Context for interrupt must be set only once"
@@ -272,7 +273,7 @@ impl WorkerMethods for Worker {
 
         // Step 3
         if let Some(cx) = self.context_for_interrupt.borrow().as_ref() {
-            cx.request_interrupt()
+            cx.request_interrupt_callback()
         }
     }
 

components/script/dom/workerglobalscope.rs

@@ -55,8 +55,7 @@ use crate::dom::workernavigator::WorkerNavigator;
 use crate::fetch;
 use crate::realms::{enter_realm, InRealm};
 use crate::script_runtime::{
-    get_reports, CommonScriptMsg, ContextForRequestInterrupt, JSContext, Runtime, ScriptChan,
-    ScriptPort,
+    get_reports, CommonScriptMsg, JSContext, Runtime, ScriptChan, ScriptPort,
 };
 use crate::task::TaskCanceller;
 use crate::task_source::dom_manipulation::DOMManipulationTaskSource;
@@ -177,10 +176,7 @@ impl WorkerGlobalScope {
     }
 
     /// Clear various items when the worker event-loop shuts-down.
-    pub fn clear_js_runtime(&self, cx_for_interrupt: ContextForRequestInterrupt) {
-        // Ensure parent thread can no longer request interrupt
-        // using our JSContext that will soon be destroyed
-        cx_for_interrupt.revoke();
+    pub fn clear_js_runtime(&self) {
         self.upcast::<GlobalScope>()
             .remove_web_messaging_and_dedicated_workers_infra();
 

components/script/script_runtime.rs

@@ -14,7 +14,7 @@ use std::io::{stdout, Write};
 use std::ops::Deref;
 use std::os::raw::c_void;
 use std::rc::Rc;
-use std::sync::{Arc, Mutex};
+use std::sync::Mutex;
 use std::time::{Duration, Instant};
 use std::{fmt, os, ptr, thread};
 
@@ -33,16 +33,16 @@ use js::jsapi::{
     InitConsumeStreamCallback, InitDispatchToEventLoop, JSContext as RawJSContext, JSGCParamKey,
     JSGCStatus, JSJitCompilerOption, JSObject, JSSecurityCallbacks, JSTracer,
     JS_AddExtraGCRootsTracer, JS_InitDestroyPrincipalsCallback, JS_InitReadPrincipalsCallback,
-    JS_RequestInterruptCallback, JS_SetGCCallback, JS_SetGCParameter,
-    JS_SetGlobalJitCompilerOption, JS_SetOffthreadIonCompilationEnabled,
-    JS_SetParallelParsingEnabled, JS_SetSecurityCallbacks, JobQueue, MimeType,
-    PromiseRejectionHandlingState, PromiseUserInputEventHandlingState, RuntimeCode,
-    SetDOMCallbacks, SetGCSliceCallback, SetJobQueue, SetPreserveWrapperCallbacks,
+    JS_SetGCCallback, JS_SetGCParameter, JS_SetGlobalJitCompilerOption,
+    JS_SetOffthreadIonCompilationEnabled, JS_SetParallelParsingEnabled, JS_SetSecurityCallbacks,
+    JobQueue, MimeType, PromiseRejectionHandlingState, PromiseUserInputEventHandlingState,
+    RuntimeCode, SetDOMCallbacks, SetGCSliceCallback, SetJobQueue, SetPreserveWrapperCallbacks,
     SetProcessBuildIdOp, SetPromiseRejectionTrackerCallback, StreamConsumer as JSStreamConsumer,
 };
 use js::jsval::UndefinedValue;
 use js::panic::wrap_panic;
 use js::rust::wrappers::{GetPromiseIsHandled, JS_GetPromiseResult};
+pub use js::rust::ThreadSafeJSContext;
 use js::rust::{
     describe_scripted_caller, Handle, HandleObject as RustHandleObject, IntoHandle, JSEngine,
     JSEngineHandle, ParentRuntime, Runtime as RustRuntime,
@@ -447,6 +447,12 @@ pub struct Runtime {
     networking_task_src: Option<Box<NetworkingTaskSource>>,
 }
 
+impl Runtime {
+    pub(crate) fn thread_safe_js_context(&self) -> ThreadSafeJSContext {
+        self.rt.thread_safe_js_context()
+    }
+}
+
 impl Drop for Runtime {
     #[allow(unsafe_code)]
     fn drop(&mut self) {
@@ -911,44 +917,6 @@ unsafe fn set_gc_zeal_options(cx: *mut RawJSContext) {
 #[cfg(not(feature = "debugmozjs"))]
 unsafe fn set_gc_zeal_options(_: *mut RawJSContext) {}
 
-/// A wrapper around a JSContext that is Send,
-/// enabling an interrupt to be requested
-/// from a thread other than the one running JS using that context.
-#[derive(Clone)]
-pub struct ContextForRequestInterrupt(Arc<Mutex<Option<*mut RawJSContext>>>);
-
-impl ContextForRequestInterrupt {
-    pub fn new(context: *mut RawJSContext) -> ContextForRequestInterrupt {
-        ContextForRequestInterrupt(Arc::new(Mutex::new(Some(context))))
-    }
-
-    pub fn revoke(&self) {
-        self.0.lock().unwrap().take();
-    }
-
-    #[allow(unsafe_code)]
-    /// Can be called from any thread, to request the callback set by
-    /// JS_AddInterruptCallback to be called on the thread
-    /// where that context is running.
-    /// The lock is held when calling JS_RequestInterruptCallback
-    /// because it is possible for the JSContext to be destroyed
-    /// on the other thread in the case of Worker shutdown
-    pub fn request_interrupt(&self) {
-        let maybe_cx = self.0.lock().unwrap();
-        if let Some(cx) = *maybe_cx {
-            unsafe {
-                JS_RequestInterruptCallback(cx);
-            }
-        }
-    }
-}
-
-#[allow(unsafe_code)]
-/// It is safe to call `JS_RequestInterruptCallback(cx)` from any thread.
-/// See the docs for the corresponding `requestInterrupt` method,
-/// at `mozjs/js/src/vm/JSContext.h`.
-unsafe impl Send for ContextForRequestInterrupt {}
-
 #[derive(Clone, Copy)]
 #[repr(transparent)]
 pub struct JSContext(*mut RawJSContext);

components/script/script_thread.rs

@@ -145,8 +145,8 @@ use crate::microtask::{Microtask, MicrotaskQueue};
 use crate::realms::enter_realm;
 use crate::script_module::ScriptFetchOptions;
 use crate::script_runtime::{
-    get_reports, new_rt_and_cx, CanGc, CommonScriptMsg, ContextForRequestInterrupt, JSContext,
-    Runtime, ScriptChan, ScriptPort, ScriptThreadEventCategory,
+    get_reports, new_rt_and_cx, CanGc, CommonScriptMsg, JSContext, Runtime, ScriptChan, ScriptPort,
+    ScriptThreadEventCategory, ThreadSafeJSContext,
 };
 use crate::task_manager::TaskManager;
 use crate::task_queue::{QueuedTask, QueuedTaskConversion, TaskQueue};
@@ -734,13 +734,13 @@ pub struct ScriptThread {
 
 struct BHMExitSignal {
     closing: Arc<AtomicBool>,
-    js_context: ContextForRequestInterrupt,
+    js_context: ThreadSafeJSContext,
 }
 
 impl BackgroundHangMonitorExitSignal for BHMExitSignal {
     fn signal_to_exit(&self) {
         self.closing.store(true, Ordering::SeqCst);
-        self.js_context.request_interrupt();
+        self.js_context.request_interrupt_callback();
     }
 }
 
@@ -1314,7 +1314,7 @@ impl ScriptThread {
         let closing = Arc::new(AtomicBool::new(false));
         let background_hang_monitor_exit_signal = BHMExitSignal {
             closing: closing.clone(),
-            js_context: ContextForRequestInterrupt::new(cx),
+            js_context: runtime.thread_safe_js_context(),
         };
 
         let background_hang_monitor = state.background_hang_monitor_register.register_component(

components/script/serviceworker_manager.rs

@@ -29,7 +29,7 @@ use crate::dom::serviceworkerglobalscope::{
     ServiceWorkerControlMsg, ServiceWorkerGlobalScope, ServiceWorkerScriptMsg,
 };
 use crate::dom::serviceworkerregistration::longest_prefix_match;
-use crate::script_runtime::{CanGc, ContextForRequestInterrupt};
+use crate::script_runtime::{CanGc, ThreadSafeJSContext};
 
 enum Message {
     FromResource(CustomResponseMediator),
@@ -103,7 +103,7 @@ impl Drop for ServiceWorkerRegistration {
         self.context
             .take()
             .expect("No context to request interrupt.")
-            .request_interrupt();
+            .request_interrupt_callback();
 
         // TODO: Step 1, 2 and 3.
         if self
@@ -134,7 +134,7 @@ struct ServiceWorkerRegistration {
     /// A handle to join on the worker thread.
     join_handle: Option<JoinHandle<()>>,
     /// A context to request an interrupt.
-    context: Option<ContextForRequestInterrupt>,
+    context: Option<ThreadSafeJSContext>,
     /// The closing flag for the worker.
     closing: Option<Arc<AtomicBool>>,
 }
@@ -157,7 +157,7 @@ impl ServiceWorkerRegistration {
         &mut self,
         join_handle: JoinHandle<()>,
         control_sender: Sender<ServiceWorkerControlMsg>,
-        context: ContextForRequestInterrupt,
+        context: ThreadSafeJSContext,
         closing: Arc<AtomicBool>,
     ) {
         assert!(self.join_handle.is_none());
@@ -454,7 +454,7 @@ fn update_serviceworker(
     ServiceWorker,
     JoinHandle<()>,
     Sender<ServiceWorkerControlMsg>,
-    ContextForRequestInterrupt,
+    ThreadSafeJSContext,
     Arc<AtomicBool>,
 ) {
     let (sender, receiver) = unbounded();