From ed688fe2c1fe004f6c4007bb08dc792a6ab6bcac Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Sun, 24 May 2020 15:39:57 +0800 Subject: [PATCH 1/5] add mechanism to join on service- and dedicated-worker threads --- components/script/dom/bindings/trace.rs | 2 ++ .../script/dom/dedicatedworkerglobalscope.rs | 6 ++-- components/script/dom/globalscope.rs | 25 ++++++++++--- .../script/dom/serviceworkerglobalscope.rs | 6 ++-- components/script/dom/worker.rs | 7 ++-- components/script/serviceworker_manager.rs | 35 ++++++++++++++++--- 6 files changed, 63 insertions(+), 18 deletions(-) diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 29160e61186..4d2496e7750 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -135,6 +135,7 @@ use std::path::PathBuf; use std::rc::Rc; use std::sync::atomic::{AtomicBool, AtomicUsize}; use std::sync::{Arc, Mutex}; +use std::thread::JoinHandle; use std::time::{Instant, SystemTime}; use style::animation::ElementAnimationSet; use style::attr::{AttrIdentifier, AttrValue, LengthOrPercentageOrAuto}; @@ -167,6 +168,7 @@ use webxr_api::SwapChainId as WebXRSwapChainId; use webxr_api::{Finger, Hand, Ray, View}; unsafe_no_jsmanaged_fields!(Tm); +unsafe_no_jsmanaged_fields!(JoinHandle<()>); /// A trait to allow tracing (only) DOM objects. pub unsafe trait JSTraceable { diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index 42f1d19e4af..685a09b5b8c 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/components/script/dom/dedicatedworkerglobalscope.rs @@ -56,7 +56,7 @@ use servo_url::ServoUrl; use std::mem::replace; use std::sync::atomic::AtomicBool; use std::sync::Arc; -use std::thread; +use std::thread::{self, JoinHandle}; use style::thread_state::{self, ThreadState}; /// Set the `worker` field of a related DedicatedWorkerGlobalScope object to a particular @@ -299,7 +299,7 @@ impl DedicatedWorkerGlobalScope { image_cache: Arc, browsing_context: Option, gpu_id_hub: Arc>, - ) { + ) -> JoinHandle<()> { let serialized_worker_url = worker_url.to_string(); let name = format!("WebWorker for {}", serialized_worker_url); let top_level_browsing_context_id = TopLevelBrowsingContextId::installed(); @@ -435,7 +435,7 @@ impl DedicatedWorkerGlobalScope { CommonScriptMsg::CollectReports, ); }) - .expect("Thread spawning failed"); + .expect("Thread spawning failed") } pub fn image_cache(&self) -> Arc { diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index 8e067e75de1..e4b024223e0 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -112,15 +112,29 @@ use std::ops::Index; use std::rc::Rc; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; +use std::thread::JoinHandle; use time::{get_time, Timespec}; use uuid::Uuid; #[derive(JSTraceable)] -pub struct AutoCloseWorker(Arc); +pub struct AutoCloseWorker { + closing: Arc, + join_handle: Option>, +} impl Drop for AutoCloseWorker { + /// fn drop(&mut self) { - self.0.store(true, Ordering::SeqCst); + // Step 1. + self.closing.store(true, Ordering::SeqCst); + + // TODO: step 2 and 3. + // Step 4 is unnecessary since we don't use actual ports for dedicated workers. + self.join_handle + .take() + .expect("No handle to join on worker.") + .join() + .expect("Couldn't join on worker thread."); } } @@ -1794,10 +1808,13 @@ impl GlobalScope { &self.permission_state_invocation_results } - pub fn track_worker(&self, closing_worker: Arc) { + pub fn track_worker(&self, closing: Arc, join_handle: JoinHandle<()>) { self.list_auto_close_worker .borrow_mut() - .push(AutoCloseWorker(closing_worker)); + .push(AutoCloseWorker { + closing, + join_handle: Some(join_handle), + }); } pub fn track_event_source(&self, event_source: &EventSource) { diff --git a/components/script/dom/serviceworkerglobalscope.rs b/components/script/dom/serviceworkerglobalscope.rs index a605e744ead..21be9240a2c 100644 --- a/components/script/dom/serviceworkerglobalscope.rs +++ b/components/script/dom/serviceworkerglobalscope.rs @@ -45,7 +45,7 @@ use servo_config::pref; use servo_rand::random; use servo_url::ServoUrl; use std::sync::Arc; -use std::thread; +use std::thread::{self, JoinHandle}; use std::time::{Duration, Instant}; use style::thread_state::{self, ThreadState}; @@ -259,7 +259,7 @@ impl ServiceWorkerGlobalScope { devtools_receiver: IpcReceiver, swmanager_sender: IpcSender, scope_url: ServoUrl, - ) { + ) -> JoinHandle<()> { let ScopeThings { script_url, init, @@ -361,7 +361,7 @@ impl ServiceWorkerGlobalScope { ); scope.clear_js_runtime(); }) - .expect("Thread spawning failed"); + .expect("Thread spawning failed") } fn handle_mixed_message(&self, msg: MixedMessage) -> bool { diff --git a/components/script/dom/worker.rs b/components/script/dom/worker.rs index ac865a0cf94..af6d9ddb23c 100644 --- a/components/script/dom/worker.rs +++ b/components/script/dom/worker.rs @@ -87,7 +87,6 @@ impl Worker { let (sender, receiver) = unbounded(); let closing = Arc::new(AtomicBool::new(false)); let worker = Worker::new(global, sender.clone(), closing.clone()); - global.track_worker(closing.clone()); let worker_ref = Trusted::new(&*worker); let worker_load_origin = WorkerScriptLoadOrigin { @@ -125,7 +124,7 @@ impl Worker { let init = prepare_workerscope_init(global, Some(devtools_sender), Some(worker_id)); - DedicatedWorkerGlobalScope::run_worker_scope( + let join_handle = DedicatedWorkerGlobalScope::run_worker_scope( init, worker_url, devtools_receiver, @@ -136,12 +135,14 @@ impl Worker { worker_load_origin, String::from(&*worker_options.name), worker_options.type_, - closing, + closing.clone(), global.image_cache(), browsing_context, global.wgpu_id_hub(), ); + global.track_worker(closing, join_handle); + Ok(worker) } diff --git a/components/script/serviceworker_manager.rs b/components/script/serviceworker_manager.rs index 7c0484144d9..b046320f371 100644 --- a/components/script/serviceworker_manager.rs +++ b/components/script/serviceworker_manager.rs @@ -24,7 +24,7 @@ use servo_config::pref; use servo_url::ImmutableOrigin; use servo_url::ServoUrl; use std::collections::HashMap; -use std::thread; +use std::thread::{self, JoinHandle}; enum Message { FromResource(CustomResponseMediator), @@ -77,6 +77,18 @@ enum RegistrationUpdateTarget { Active, } +impl Drop for ServiceWorkerRegistration { + /// + fn drop(&mut self) { + // TODO: Step 1, 2 and 3. + self.join_handle + .take() + .expect("No handle to join on worker.") + .join() + .expect("Couldn't join on worker thread."); + } +} + /// https://w3c.github.io/ServiceWorker/#service-worker-registration-concept struct ServiceWorkerRegistration { /// A unique identifer. @@ -87,6 +99,8 @@ struct ServiceWorkerRegistration { waiting_worker: Option, /// https://w3c.github.io/ServiceWorker/#dfn-installing-worker installing_worker: Option, + /// A handle to join on the worker thread. + join_handle: Option>, } impl ServiceWorkerRegistration { @@ -96,9 +110,15 @@ impl ServiceWorkerRegistration { active_worker: None, waiting_worker: None, installing_worker: None, + join_handle: None, } } + fn note_worker_thread(&mut self, join_handle: JoinHandle<()>) { + assert!(self.join_handle.is_none()); + self.join_handle = Some(join_handle); + } + /// fn get_newest_worker(&self) -> Option { if let Some(worker) = self.active_worker.as_ref() { @@ -326,9 +346,11 @@ impl ServiceWorkerManager { // Very roughly steps 5 to 18. // TODO: implement all steps precisely. - let new_worker = + let (new_worker, join_handle) = update_serviceworker(self.own_sender.clone(), job.scope_url.clone(), scope_things); + registration.note_worker_thread(join_handle); + // Step 19, run Install. // Install: Step 4, run Update Registration State. @@ -363,12 +385,12 @@ fn update_serviceworker( own_sender: IpcSender, scope_url: ServoUrl, scope_things: ScopeThings, -) -> ServiceWorker { +) -> (ServiceWorker, JoinHandle<()>) { let (sender, receiver) = unbounded(); let (_devtools_sender, devtools_receiver) = ipc::channel().unwrap(); let worker_id = ServiceWorkerId::new(); - ServiceWorkerGlobalScope::run_serviceworker_scope( + let join_handle = ServiceWorkerGlobalScope::run_serviceworker_scope( scope_things.clone(), sender.clone(), receiver, @@ -377,7 +399,10 @@ fn update_serviceworker( scope_url.clone(), ); - ServiceWorker::new(scope_things.script_url, sender, worker_id) + ( + ServiceWorker::new(scope_things.script_url, sender, worker_id), + join_handle, + ) } impl ServiceWorkerManagerFactory for ServiceWorkerManager { From 947fa8bbb7195861a1d719422b0e961eda857eaf Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Sun, 24 May 2020 16:36:10 +0800 Subject: [PATCH 2/5] add a control chan to workers, use to signal shutdown --- .../script/dom/abstractworkerglobalscope.rs | 11 +++- .../script/dom/dedicatedworkerglobalscope.rs | 36 +++++++++++- components/script/dom/globalscope.rs | 34 ++++++++++-- .../script/dom/serviceworkerglobalscope.rs | 55 ++++++++++++++----- components/script/dom/worker.rs | 5 +- components/script/serviceworker_manager.rs | 55 ++++++++++++++++--- 6 files changed, 165 insertions(+), 31 deletions(-) diff --git a/components/script/dom/abstractworkerglobalscope.rs b/components/script/dom/abstractworkerglobalscope.rs index 336bf1996e7..ca994c1c694 100644 --- a/components/script/dom/abstractworkerglobalscope.rs +++ b/components/script/dom/abstractworkerglobalscope.rs @@ -82,12 +82,15 @@ impl ScriptPort for Receiver { pub trait WorkerEventLoopMethods { type WorkerMsg: QueuedTaskConversion + Send; + type ControlMsg; type Event; fn task_queue(&self) -> &TaskQueue; - fn handle_event(&self, event: Self::Event); + fn handle_event(&self, event: Self::Event) -> bool; fn handle_worker_post_event(&self, worker: &TrustedWorkerAddress) -> Option; + fn from_control_msg(&self, msg: Self::ControlMsg) -> Self::Event; fn from_worker_msg(&self, msg: Self::WorkerMsg) -> Self::Event; fn from_devtools_msg(&self, msg: DevtoolScriptControlMsg) -> Self::Event; + fn control_receiver(&self) -> &Receiver; } // https://html.spec.whatwg.org/multipage/#worker-event-loop @@ -108,6 +111,7 @@ pub fn run_worker_event_loop( }; let task_queue = worker_scope.task_queue(); let event = select! { + recv(worker_scope.control_receiver()) -> msg => worker_scope.from_control_msg(msg.unwrap()), recv(task_queue.select()) -> msg => { task_queue.take_tasks(msg.unwrap()); worker_scope.from_worker_msg(task_queue.recv().unwrap()) @@ -136,7 +140,10 @@ pub fn run_worker_event_loop( } // Step 3 for event in sequential { - worker_scope.handle_event(event); + if !worker_scope.handle_event(event) { + // Shutdown + return; + } // Step 6 let _ar = match worker { Some(worker) => worker_scope.handle_worker_post_event(worker), diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index 685a09b5b8c..edd9d6b9480 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/components/script/dom/dedicatedworkerglobalscope.rs @@ -86,6 +86,12 @@ impl<'a> Drop for AutoWorkerReset<'a> { } } +/// Messages sent from the owning global. +pub enum DedicatedWorkerControlMsg { + /// Shutdown the worker. + Exit, +} + pub enum DedicatedWorkerScriptMsg { /// Standard message from a worker. CommonWorker(TrustedWorkerAddress, WorkerScriptMsg), @@ -96,6 +102,7 @@ pub enum DedicatedWorkerScriptMsg { pub enum MixedMessage { FromWorker(DedicatedWorkerScriptMsg), FromDevtools(DevtoolScriptControlMsg), + FromControl(DedicatedWorkerControlMsg), } impl QueuedTaskConversion for DedicatedWorkerScriptMsg { @@ -183,18 +190,23 @@ pub struct DedicatedWorkerGlobalScope { #[ignore_malloc_size_of = "Arc"] image_cache: Arc, browsing_context: Option, + /// A receiver of control messages, + /// currently only used to signal shutdown. + #[ignore_malloc_size_of = "Channels are hard"] + control_receiver: Receiver, } impl WorkerEventLoopMethods for DedicatedWorkerGlobalScope { type WorkerMsg = DedicatedWorkerScriptMsg; + type ControlMsg = DedicatedWorkerControlMsg; type Event = MixedMessage; fn task_queue(&self) -> &TaskQueue { &self.task_queue } - fn handle_event(&self, event: MixedMessage) { - self.handle_mixed_message(event); + fn handle_event(&self, event: MixedMessage) -> bool { + self.handle_mixed_message(event) } fn handle_worker_post_event(&self, worker: &TrustedWorkerAddress) -> Option { @@ -202,6 +214,10 @@ impl WorkerEventLoopMethods for DedicatedWorkerGlobalScope { Some(ar) } + fn from_control_msg(&self, msg: DedicatedWorkerControlMsg) -> MixedMessage { + MixedMessage::FromControl(msg) + } + fn from_worker_msg(&self, msg: DedicatedWorkerScriptMsg) -> MixedMessage { MixedMessage::FromWorker(msg) } @@ -209,6 +225,10 @@ impl WorkerEventLoopMethods for DedicatedWorkerGlobalScope { fn from_devtools_msg(&self, msg: DevtoolScriptControlMsg) -> MixedMessage { MixedMessage::FromDevtools(msg) } + + fn control_receiver(&self) -> &Receiver { + &self.control_receiver + } } impl DedicatedWorkerGlobalScope { @@ -226,6 +246,7 @@ impl DedicatedWorkerGlobalScope { image_cache: Arc, browsing_context: Option, gpu_id_hub: Arc>, + control_receiver: Receiver, ) -> DedicatedWorkerGlobalScope { DedicatedWorkerGlobalScope { workerglobalscope: WorkerGlobalScope::new_inherited( @@ -244,6 +265,7 @@ impl DedicatedWorkerGlobalScope { worker: DomRefCell::new(None), image_cache: image_cache, browsing_context, + control_receiver, } } @@ -262,6 +284,7 @@ impl DedicatedWorkerGlobalScope { image_cache: Arc, browsing_context: Option, gpu_id_hub: Arc>, + control_receiver: Receiver, ) -> DomRoot { let cx = runtime.cx(); let scope = Box::new(DedicatedWorkerGlobalScope::new_inherited( @@ -278,6 +301,7 @@ impl DedicatedWorkerGlobalScope { image_cache, browsing_context, gpu_id_hub, + control_receiver, )); unsafe { DedicatedWorkerGlobalScopeBinding::Wrap(SafeJSContext::from_ptr(cx), scope) } } @@ -299,6 +323,7 @@ impl DedicatedWorkerGlobalScope { image_cache: Arc, browsing_context: Option, gpu_id_hub: Arc>, + control_receiver: Receiver, ) -> JoinHandle<()> { let serialized_worker_url = worker_url.to_string(); let name = format!("WebWorker for {}", serialized_worker_url); @@ -370,6 +395,7 @@ impl DedicatedWorkerGlobalScope { image_cache, browsing_context, gpu_id_hub, + control_receiver, ); // FIXME(njn): workers currently don't have a unique ID suitable for using in reporter // registration (#6631), so we instead use a random number and cross our fingers. @@ -485,7 +511,7 @@ impl DedicatedWorkerGlobalScope { } } - fn handle_mixed_message(&self, msg: MixedMessage) { + fn handle_mixed_message(&self, msg: MixedMessage) -> bool { // FIXME(#26324): `self.worker` is None in devtools messages. match msg { MixedMessage::FromDevtools(msg) => match msg { @@ -505,7 +531,11 @@ impl DedicatedWorkerGlobalScope { self.handle_script_event(msg); }, MixedMessage::FromWorker(DedicatedWorkerScriptMsg::WakeUp) => {}, + MixedMessage::FromControl(DedicatedWorkerControlMsg::Exit) => { + return false; + }, } + true } // https://html.spec.whatwg.org/multipage/#runtime-script-errors-2 diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index e4b024223e0..1a66e4d94f9 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -26,7 +26,9 @@ use crate::dom::bindings::weakref::{DOMTracker, WeakRef}; use crate::dom::blob::Blob; use crate::dom::broadcastchannel::BroadcastChannel; use crate::dom::crypto::Crypto; -use crate::dom::dedicatedworkerglobalscope::DedicatedWorkerGlobalScope; +use crate::dom::dedicatedworkerglobalscope::{ + DedicatedWorkerControlMsg, DedicatedWorkerGlobalScope, +}; use crate::dom::errorevent::ErrorEvent; use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus}; use crate::dom::eventsource::EventSource; @@ -65,6 +67,7 @@ use crate::task_source::TaskSourceName; use crate::timers::{IsInterval, OneshotTimerCallback, OneshotTimerHandle}; use crate::timers::{OneshotTimers, TimerCallback}; use content_security_policy::CspList; +use crossbeam_channel::Sender; use devtools_traits::{PageError, ScriptToDevtoolsControlMsg}; use dom_struct::dom_struct; use embedder_traits::EmbedderMsg; @@ -118,8 +121,13 @@ use uuid::Uuid; #[derive(JSTraceable)] pub struct AutoCloseWorker { + /// https://html.spec.whatwg.org/multipage/#dom-workerglobalscope-closing closing: Arc, + /// A handle to join on the worker thread. join_handle: Option>, + /// A sender of control messages, + /// currently only used to signal shutdown. + control_sender: Sender, } impl Drop for AutoCloseWorker { @@ -128,13 +136,25 @@ impl Drop for AutoCloseWorker { // Step 1. self.closing.store(true, Ordering::SeqCst); + if self + .control_sender + .send(DedicatedWorkerControlMsg::Exit) + .is_err() + { + warn!("Couldn't send an exit message to a dedicated worker."); + } + // TODO: step 2 and 3. // Step 4 is unnecessary since we don't use actual ports for dedicated workers. - self.join_handle + if self + .join_handle .take() .expect("No handle to join on worker.") .join() - .expect("Couldn't join on worker thread."); + .is_err() + { + warn!("Failed to join on dedicated worker thread."); + } } } @@ -1808,12 +1828,18 @@ impl GlobalScope { &self.permission_state_invocation_results } - pub fn track_worker(&self, closing: Arc, join_handle: JoinHandle<()>) { + pub fn track_worker( + &self, + closing: Arc, + join_handle: JoinHandle<()>, + control_sender: Sender, + ) { self.list_auto_close_worker .borrow_mut() .push(AutoCloseWorker { closing, join_handle: Some(join_handle), + control_sender: control_sender, }); } diff --git a/components/script/dom/serviceworkerglobalscope.rs b/components/script/dom/serviceworkerglobalscope.rs index 21be9240a2c..f3d11a6c827 100644 --- a/components/script/dom/serviceworkerglobalscope.rs +++ b/components/script/dom/serviceworkerglobalscope.rs @@ -116,9 +116,16 @@ impl QueuedTaskConversion for ServiceWorkerScriptMsg { } } +/// Messages sent from the owning registration. +pub enum ServiceWorkerControlMsg { + /// Shutdown. + Exit, +} + pub enum MixedMessage { FromServiceWorker(ServiceWorkerScriptMsg), FromDevtools(DevtoolScriptControlMsg), + FromControl(ServiceWorkerControlMsg), } #[derive(Clone, JSTraceable)] @@ -165,24 +172,34 @@ pub struct ServiceWorkerGlobalScope { swmanager_sender: IpcSender, scope_url: ServoUrl, + + /// A receiver of control messages, + /// currently only used to signal shutdown. + #[ignore_malloc_size_of = "Channels are hard"] + control_receiver: Receiver, } impl WorkerEventLoopMethods for ServiceWorkerGlobalScope { type WorkerMsg = ServiceWorkerScriptMsg; + type ControlMsg = ServiceWorkerControlMsg; type Event = MixedMessage; fn task_queue(&self) -> &TaskQueue { &self.task_queue } - fn handle_event(&self, event: MixedMessage) { - self.handle_mixed_message(event); + fn handle_event(&self, event: MixedMessage) -> bool { + self.handle_mixed_message(event) } fn handle_worker_post_event(&self, _worker: &TrustedWorkerAddress) -> Option { None } + fn from_control_msg(&self, msg: ServiceWorkerControlMsg) -> MixedMessage { + MixedMessage::FromControl(msg) + } + fn from_worker_msg(&self, msg: ServiceWorkerScriptMsg) -> MixedMessage { MixedMessage::FromServiceWorker(msg) } @@ -190,6 +207,10 @@ impl WorkerEventLoopMethods for ServiceWorkerGlobalScope { fn from_devtools_msg(&self, msg: DevtoolScriptControlMsg) -> MixedMessage { MixedMessage::FromDevtools(msg) } + + fn control_receiver(&self) -> &Receiver { + &self.control_receiver + } } impl ServiceWorkerGlobalScope { @@ -203,6 +224,7 @@ impl ServiceWorkerGlobalScope { time_out_port: Receiver, swmanager_sender: IpcSender, scope_url: ServoUrl, + control_receiver: Receiver, ) -> ServiceWorkerGlobalScope { ServiceWorkerGlobalScope { workerglobalscope: WorkerGlobalScope::new_inherited( @@ -220,6 +242,7 @@ impl ServiceWorkerGlobalScope { time_out_port, swmanager_sender: swmanager_sender, scope_url: scope_url, + control_receiver, } } @@ -234,6 +257,7 @@ impl ServiceWorkerGlobalScope { time_out_port: Receiver, swmanager_sender: IpcSender, scope_url: ServoUrl, + control_receiver: Receiver, ) -> DomRoot { let cx = runtime.cx(); let scope = Box::new(ServiceWorkerGlobalScope::new_inherited( @@ -246,6 +270,7 @@ impl ServiceWorkerGlobalScope { time_out_port, swmanager_sender, scope_url, + control_receiver, )); unsafe { ServiceWorkerGlobalScopeBinding::Wrap(SafeJSContext::from_ptr(cx), scope) } } @@ -259,6 +284,7 @@ impl ServiceWorkerGlobalScope { devtools_receiver: IpcReceiver, swmanager_sender: IpcSender, scope_url: ServoUrl, + control_receiver: Receiver, ) -> JoinHandle<()> { let ScopeThings { script_url, @@ -315,6 +341,7 @@ impl ServiceWorkerGlobalScope { time_out_port, swmanager_sender, scope_url, + control_receiver, ); let (_url, source) = @@ -366,23 +393,23 @@ impl ServiceWorkerGlobalScope { fn handle_mixed_message(&self, msg: MixedMessage) -> bool { match msg { - MixedMessage::FromDevtools(msg) => { - match msg { - DevtoolScriptControlMsg::EvaluateJS(_pipe_id, string, sender) => { - devtools::handle_evaluate_js(self.upcast(), string, sender) - }, - DevtoolScriptControlMsg::WantsLiveNotifications(_pipe_id, bool_val) => { - devtools::handle_wants_live_notifications(self.upcast(), bool_val) - }, - _ => debug!("got an unusable devtools control message inside the worker!"), - } - true + MixedMessage::FromDevtools(msg) => match msg { + DevtoolScriptControlMsg::EvaluateJS(_pipe_id, string, sender) => { + devtools::handle_evaluate_js(self.upcast(), string, sender) + }, + DevtoolScriptControlMsg::WantsLiveNotifications(_pipe_id, bool_val) => { + devtools::handle_wants_live_notifications(self.upcast(), bool_val) + }, + _ => debug!("got an unusable devtools control message inside the worker!"), }, MixedMessage::FromServiceWorker(msg) => { self.handle_script_event(msg); - true + }, + MixedMessage::FromControl(ServiceWorkerControlMsg::Exit) => { + return false; }, } + true } fn has_timed_out(&self) -> bool { diff --git a/components/script/dom/worker.rs b/components/script/dom/worker.rs index af6d9ddb23c..ca3b811b042 100644 --- a/components/script/dom/worker.rs +++ b/components/script/dom/worker.rs @@ -124,6 +124,8 @@ impl Worker { let init = prepare_workerscope_init(global, Some(devtools_sender), Some(worker_id)); + let (control_sender, control_receiver) = unbounded(); + let join_handle = DedicatedWorkerGlobalScope::run_worker_scope( init, worker_url, @@ -139,9 +141,10 @@ impl Worker { global.image_cache(), browsing_context, global.wgpu_id_hub(), + control_receiver, ); - global.track_worker(closing, join_handle); + global.track_worker(closing, join_handle, control_sender); Ok(worker) } diff --git a/components/script/serviceworker_manager.rs b/components/script/serviceworker_manager.rs index b046320f371..1cbca40e8cc 100644 --- a/components/script/serviceworker_manager.rs +++ b/components/script/serviceworker_manager.rs @@ -8,7 +8,9 @@ //! active_workers map use crate::dom::abstractworker::WorkerScriptMsg; -use crate::dom::serviceworkerglobalscope::{ServiceWorkerGlobalScope, ServiceWorkerScriptMsg}; +use crate::dom::serviceworkerglobalscope::{ + ServiceWorkerControlMsg, ServiceWorkerGlobalScope, ServiceWorkerScriptMsg, +}; use crate::dom::serviceworkerregistration::longest_prefix_match; use crossbeam_channel::{unbounded, Receiver, RecvError, Sender}; use ipc_channel::ipc::{self, IpcSender}; @@ -80,12 +82,27 @@ enum RegistrationUpdateTarget { impl Drop for ServiceWorkerRegistration { /// fn drop(&mut self) { + // Drop the channel to signal shutdown. + if self + .control_sender + .take() + .expect("No control sender to worker thread.") + .send(ServiceWorkerControlMsg::Exit) + .is_err() + { + warn!("Failed to send exit message to service worker scope."); + } + // TODO: Step 1, 2 and 3. - self.join_handle + if self + .join_handle .take() .expect("No handle to join on worker.") .join() - .expect("Couldn't join on worker thread."); + .is_err() + { + warn!("Failed to join on service worker thread."); + } } } @@ -99,6 +116,9 @@ struct ServiceWorkerRegistration { waiting_worker: Option, /// https://w3c.github.io/ServiceWorker/#dfn-installing-worker installing_worker: Option, + /// A channel to send control message to the worker, + /// currently only used to signal shutdown. + control_sender: Option>, /// A handle to join on the worker thread. join_handle: Option>, } @@ -111,12 +131,20 @@ impl ServiceWorkerRegistration { waiting_worker: None, installing_worker: None, join_handle: None, + control_sender: None, } } - fn note_worker_thread(&mut self, join_handle: JoinHandle<()>) { + fn note_worker_thread( + &mut self, + join_handle: JoinHandle<()>, + control_sender: Sender, + ) { assert!(self.join_handle.is_none()); self.join_handle = Some(join_handle); + + assert!(self.control_sender.is_none()); + self.control_sender = Some(control_sender); } /// @@ -203,6 +231,10 @@ impl ServiceWorkerManager { Message::FromResource(msg) => self.handle_message_from_resource(msg), }; if !should_continue { + for registration in self.registrations.drain() { + // Signal shut-down, and join on the thread. + drop(registration); + } break; } } @@ -346,10 +378,11 @@ impl ServiceWorkerManager { // Very roughly steps 5 to 18. // TODO: implement all steps precisely. - let (new_worker, join_handle) = + let (new_worker, join_handle, control_sender) = update_serviceworker(self.own_sender.clone(), job.scope_url.clone(), scope_things); - registration.note_worker_thread(join_handle); + // Since we've just started the worker thread, ensure we can shut it down later. + registration.note_worker_thread(join_handle, control_sender); // Step 19, run Install. @@ -385,11 +418,17 @@ fn update_serviceworker( own_sender: IpcSender, scope_url: ServoUrl, scope_things: ScopeThings, -) -> (ServiceWorker, JoinHandle<()>) { +) -> ( + ServiceWorker, + JoinHandle<()>, + Sender, +) { let (sender, receiver) = unbounded(); let (_devtools_sender, devtools_receiver) = ipc::channel().unwrap(); let worker_id = ServiceWorkerId::new(); + let (control_sender, control_receiver) = unbounded(); + let join_handle = ServiceWorkerGlobalScope::run_serviceworker_scope( scope_things.clone(), sender.clone(), @@ -397,11 +436,13 @@ fn update_serviceworker( devtools_receiver, own_sender, scope_url.clone(), + control_receiver, ); ( ServiceWorker::new(scope_things.script_url, sender, worker_id), join_handle, + control_sender, ) } From 517473924405655e6c9dbecdd8301a9932b5289c Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Sun, 24 May 2020 16:40:57 +0800 Subject: [PATCH 3/5] turn serviceworker event-loop back on --- components/script/dom/serviceworkerglobalscope.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/components/script/dom/serviceworkerglobalscope.rs b/components/script/dom/serviceworkerglobalscope.rs index f3d11a6c827..840ee81f764 100644 --- a/components/script/dom/serviceworkerglobalscope.rs +++ b/components/script/dom/serviceworkerglobalscope.rs @@ -414,9 +414,7 @@ impl ServiceWorkerGlobalScope { fn has_timed_out(&self) -> bool { // TODO: https://w3c.github.io/ServiceWorker/#service-worker-lifetime - // Since we don't have a shutdown mechanism yet, see #26548 - // immediately stop the event-loop after executing the initial script. - true + false } fn handle_script_event(&self, msg: ServiceWorkerScriptMsg) { From 6f34b52e3999da7bf345cdc5e8b9ce8752b499f5 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Fri, 29 May 2020 18:12:02 +0800 Subject: [PATCH 4/5] properly shutdown dedicated workers when the owning scope shuts-down --- components/script/dom/dedicatedworkerglobalscope.rs | 1 + components/script/dom/globalscope.rs | 11 ++++++++++- components/script/dom/window.rs | 4 ++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/components/script/dom/dedicatedworkerglobalscope.rs b/components/script/dom/dedicatedworkerglobalscope.rs index edd9d6b9480..c764678b80a 100644 --- a/components/script/dom/dedicatedworkerglobalscope.rs +++ b/components/script/dom/dedicatedworkerglobalscope.rs @@ -460,6 +460,7 @@ impl DedicatedWorkerGlobalScope { parent_sender, CommonScriptMsg::CollectReports, ); + scope.clear_js_runtime(); }) .expect("Thread spawning failed") } diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index 1a66e4d94f9..d03333e8541 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -794,9 +794,18 @@ impl GlobalScope { } /// Remove the routers for ports and broadcast-channels. - pub fn remove_web_messaging_infra(&self) { + /// Drain the list of workers. + pub fn remove_web_messaging_and_dedicated_workers_infra(&self) { self.remove_message_ports_router(); self.remove_broadcast_channel_router(); + + // Drop each ref to a worker explicitly now, + // which will send a shutdown signal, + // and join on the worker thread. + self.list_auto_close_worker + .borrow_mut() + .drain(0..) + .for_each(|worker| drop(worker)); } /// Update our state to un-managed, diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 16126dcf1b9..d7308c7462d 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1441,8 +1441,8 @@ impl Window { } pub fn clear_js_runtime(&self) { - // Remove the infra for managing messageports and broadcast channels. - self.upcast::().remove_web_messaging_infra(); + self.upcast::() + .remove_web_messaging_and_dedicated_workers_infra(); // Clean up any active promises // https://github.com/servo/servo/issues/15318 From f4d258d6742ad1898aa97f24010d5c14891b9083 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Fri, 29 May 2020 23:08:46 +0800 Subject: [PATCH 5/5] remove messagaging and worker infra on workerscope exits --- components/script/dom/workerglobalscope.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/script/dom/workerglobalscope.rs b/components/script/dom/workerglobalscope.rs index 8d54c3f7da4..9cb99b7d154 100644 --- a/components/script/dom/workerglobalscope.rs +++ b/components/script/dom/workerglobalscope.rs @@ -161,7 +161,12 @@ impl WorkerGlobalScope { } } + /// Clear various items when the worker event-loop shuts-down. pub fn clear_js_runtime(&self) { + self.upcast::() + .remove_web_messaging_and_dedicated_workers_infra(); + + // Drop the runtime. let runtime = self.runtime.borrow_mut().take(); drop(runtime); }