diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index 6d1792f87c2..acd2942b1a8 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/components/script/dom/dedicatedworkerglobalscope.rs @@ -378,7 +378,8 @@ impl DedicatedWorkerGlobalScope { new_child_runtime(parent, Some(task_source)) }; - let _ = context_sender.send(ContextForRequestInterrupt::new(runtime.cx())); + let context_for_interrupt = ContextForRequestInterrupt::new(runtime.cx()); + let _ = context_sender.send(context_for_interrupt.clone()); let (devtools_mpsc_chan, devtools_mpsc_port) = unbounded(); ROUTER.route_ipc_receiver_to_crossbeam_sender( @@ -476,7 +477,8 @@ impl DedicatedWorkerGlobalScope { parent_sender, CommonScriptMsg::CollectReports, ); - scope.clear_js_runtime(); + + scope.clear_js_runtime(context_for_interrupt); }) .expect("Thread spawning failed") } diff --git a/components/script/dom/serviceworkerglobalscope.rs b/components/script/dom/serviceworkerglobalscope.rs index 934b9b88b3a..cbd38c0fc42 100644 --- a/components/script/dom/serviceworkerglobalscope.rs +++ b/components/script/dom/serviceworkerglobalscope.rs @@ -306,7 +306,8 @@ impl ServiceWorkerGlobalScope { .spawn(move || { thread_state::initialize(ThreadState::SCRIPT | ThreadState::IN_WORKER); let runtime = new_rt_and_cx(None); - let _ = context_sender.send(ContextForRequestInterrupt::new(runtime.cx())); + let context_for_interrupt = ContextForRequestInterrupt::new(runtime.cx()); + let _ = context_sender.send(context_for_interrupt.clone()); let roots = RootCollection::new(); let _stack_roots = ThreadLocalStackRoots::new(&roots); @@ -396,7 +397,8 @@ impl ServiceWorkerGlobalScope { scope.script_chan(), CommonScriptMsg::CollectReports, ); - scope.clear_js_runtime(); + + scope.clear_js_runtime(context_for_interrupt); }) .expect("Thread spawning failed") } diff --git a/components/script/dom/worker.rs b/components/script/dom/worker.rs index 569f8188802..fbc0c0cd138 100644 --- a/components/script/dom/worker.rs +++ b/components/script/dom/worker.rs @@ -4,6 +4,7 @@ use crate::dom::abstractworker::SimpleWorkerErrorHandler; use crate::dom::abstractworker::WorkerScriptMsg; +use crate::dom::bindings::cell::DomRefCell; use crate::dom::bindings::codegen::Bindings::MessagePortBinding::PostMessageOptions; use crate::dom::bindings::codegen::Bindings::WorkerBinding::{WorkerMethods, WorkerOptions}; use crate::dom::bindings::error::{Error, ErrorResult, Fallible}; @@ -23,13 +24,13 @@ 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::JSContext; +use crate::script_runtime::{ContextForRequestInterrupt, JSContext}; use crate::task::TaskOnce; use crossbeam_channel::{unbounded, Sender}; use devtools_traits::{DevtoolsPageInfo, ScriptToDevtoolsControlMsg, WorkerId}; use dom_struct::dom_struct; use ipc_channel::ipc; -use js::jsapi::{Heap, JSObject, JS_RequestInterruptCallback}; +use js::jsapi::{Heap, JSObject}; use js::jsval::UndefinedValue; use js::rust::{CustomAutoRooter, CustomAutoRooterGuard, HandleObject, HandleValue}; use script_traits::{StructuredSerializedData, WorkerScriptLoadOrigin}; @@ -51,6 +52,8 @@ pub struct Worker { #[ignore_malloc_size_of = "Arc"] closing: Arc, terminated: Cell, + #[ignore_malloc_size_of = "Arc"] + context_for_interrupt: DomRefCell>, } impl Worker { @@ -60,6 +63,7 @@ impl Worker { sender: sender, closing: closing, terminated: Cell::new(false), + context_for_interrupt: Default::default(), } } @@ -156,6 +160,7 @@ impl Worker { .recv() .expect("Couldn't receive a context for worker."); + worker.set_context_for_interrupt(context.clone()); global.track_worker(closing, join_handle, control_sender, context); Ok(worker) @@ -165,6 +170,14 @@ impl Worker { self.terminated.get() } + pub fn set_context_for_interrupt(&self, cx: ContextForRequestInterrupt) { + assert!( + self.context_for_interrupt.borrow().is_none(), + "Context for interrupt must be set only once" + ); + *self.context_for_interrupt.borrow_mut() = Some(cx); + } + pub fn handle_message(address: TrustedWorkerAddress, data: StructuredSerializedData) { let worker = address.root(); @@ -241,7 +254,6 @@ impl WorkerMethods for Worker { self.post_message_impl(cx, message, guard) } - #[allow(unsafe_code)] // https://html.spec.whatwg.org/multipage/#terminate-a-worker fn Terminate(&self) { // Step 1 @@ -253,8 +265,10 @@ impl WorkerMethods for Worker { self.terminated.set(true); // Step 3 - let cx = GlobalScope::get_cx(); - unsafe { JS_RequestInterruptCallback(*cx) }; + self.context_for_interrupt + .borrow() + .as_ref() + .map(|cx| cx.request_interrupt()); } // https://html.spec.whatwg.org/multipage/#handler-worker-onmessage diff --git a/components/script/dom/workerglobalscope.rs b/components/script/dom/workerglobalscope.rs index 6092b3c4c3e..3aaeca56d3b 100644 --- a/components/script/dom/workerglobalscope.rs +++ b/components/script/dom/workerglobalscope.rs @@ -30,8 +30,8 @@ use crate::dom::workerlocation::WorkerLocation; use crate::dom::workernavigator::WorkerNavigator; use crate::fetch; use crate::realms::{enter_realm, InRealm}; -use crate::script_runtime::JSContext; use crate::script_runtime::{get_reports, CommonScriptMsg, Runtime, ScriptChan, ScriptPort}; +use crate::script_runtime::{ContextForRequestInterrupt, JSContext}; use crate::task::TaskCanceller; use crate::task_source::dom_manipulation::DOMManipulationTaskSource; use crate::task_source::file_reading::FileReadingTaskSource; @@ -166,7 +166,10 @@ impl WorkerGlobalScope { } /// Clear various items when the worker event-loop shuts-down. - pub fn clear_js_runtime(&self) { + 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(); self.upcast::() .remove_web_messaging_and_dedicated_workers_infra(); @@ -283,8 +286,14 @@ impl WorkerGlobalScopeMethods for WorkerGlobalScope { match result { Ok(_) => (), Err(_) => { - println!("evaluate_script failed"); - return Err(Error::JSFailed); + if self.is_closing() { + // Don't return JSFailed as we might not have + // any pending exceptions. + println!("evaluate_script failed (terminated)"); + } else { + println!("evaluate_script failed"); + return Err(Error::JSFailed); + } }, } } diff --git a/components/script/script_runtime.rs b/components/script/script_runtime.rs index 59cbdc77ba3..709b2891306 100644 --- a/components/script/script_runtime.rs +++ b/components/script/script_runtime.rs @@ -87,7 +87,7 @@ use std::os; use std::os::raw::c_void; use std::ptr; use std::rc::Rc; -use std::sync::Mutex; +use std::sync::{Arc, Mutex}; use std::thread; use std::time::Duration; use style::thread_state::{self, ThreadState}; @@ -858,24 +858,34 @@ unsafe fn set_gc_zeal_options(cx: *mut RawJSContext) { #[cfg(not(feature = "debugmozjs"))] unsafe fn set_gc_zeal_options(_: *mut RawJSContext) {} -#[repr(transparent)] /// 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. -pub struct ContextForRequestInterrupt(*mut RawJSContext); +#[derive(Clone)] +pub struct ContextForRequestInterrupt(Arc>>); impl ContextForRequestInterrupt { pub fn new(context: *mut RawJSContext) -> ContextForRequestInterrupt { - ContextForRequestInterrupt(context) + 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. + /// 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) { - unsafe { - JS_RequestInterruptCallback(self.0); + let maybe_cx = self.0.lock().unwrap(); + if let Some(cx) = *maybe_cx { + unsafe { + JS_RequestInterruptCallback(cx); + } } } }