Auto merge of #26628 - gterzian:shutdown_workers, r=asajeffrey

Improve worker shutdown

<!-- Please describe your changes on the following line: -->
FIX #26548
FIX #25212

and also  a step towards https://github.com/servo/servo/issues/26502

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

components/script/dom/abstractworkerglobalscope.rs

@@ -82,12 +82,15 @@ impl ScriptPort for Receiver<DedicatedWorkerScriptMsg> {
 
 pub trait WorkerEventLoopMethods {
     type WorkerMsg: QueuedTaskConversion + Send;
+    type ControlMsg;
     type Event;
     fn task_queue(&self) -> &TaskQueue<Self::WorkerMsg>;
-    fn handle_event(&self, event: Self::Event);
+    fn handle_event(&self, event: Self::Event) -> bool;
     fn handle_worker_post_event(&self, worker: &TrustedWorkerAddress) -> Option<AutoWorkerReset>;
+    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<Self::ControlMsg>;
 }
 
 // https://html.spec.whatwg.org/multipage/#worker-event-loop
@@ -108,6 +111,7 @@ pub fn run_worker_event_loop<T, WorkerMsg, Event>(
     };
     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<T, WorkerMsg, Event>(
     }
     // 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),

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};
@@ -168,6 +169,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 {

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
@@ -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<dyn ImageCache>,
     browsing_context: Option<BrowsingContextId>,
+    /// A receiver of control messages,
+    /// currently only used to signal shutdown.
+    #[ignore_malloc_size_of = "Channels are hard"]
+    control_receiver: Receiver<DedicatedWorkerControlMsg>,
 }
 
 impl WorkerEventLoopMethods for DedicatedWorkerGlobalScope {
     type WorkerMsg = DedicatedWorkerScriptMsg;
+    type ControlMsg = DedicatedWorkerControlMsg;
     type Event = MixedMessage;
 
     fn task_queue(&self) -> &TaskQueue<DedicatedWorkerScriptMsg> {
         &self.task_queue
     }
 
-    fn handle_event(&self, event: MixedMessage) {
+    fn handle_event(&self, event: MixedMessage) -> bool {
-        self.handle_mixed_message(event);
+        self.handle_mixed_message(event)
     }
 
     fn handle_worker_post_event(&self, worker: &TrustedWorkerAddress) -> Option<AutoWorkerReset> {
@@ -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<DedicatedWorkerControlMsg> {
+        &self.control_receiver
+    }
 }
 
 impl DedicatedWorkerGlobalScope {
@@ -226,6 +246,7 @@ impl DedicatedWorkerGlobalScope {
         image_cache: Arc<dyn ImageCache>,
         browsing_context: Option<BrowsingContextId>,
         gpu_id_hub: Arc<Mutex<Identities>>,
+        control_receiver: Receiver<DedicatedWorkerControlMsg>,
     ) -> 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<dyn ImageCache>,
         browsing_context: Option<BrowsingContextId>,
         gpu_id_hub: Arc<Mutex<Identities>>,
+        control_receiver: Receiver<DedicatedWorkerControlMsg>,
     ) -> DomRoot<DedicatedWorkerGlobalScope> {
         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,7 +323,8 @@ impl DedicatedWorkerGlobalScope {
         image_cache: Arc<dyn ImageCache>,
         browsing_context: Option<BrowsingContextId>,
         gpu_id_hub: Arc<Mutex<Identities>>,
-    ) {
+        control_receiver: Receiver<DedicatedWorkerControlMsg>,
+    ) -> 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();
@@ -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.
@@ -434,8 +460,9 @@ impl DedicatedWorkerGlobalScope {
                         parent_sender,
                         CommonScriptMsg::CollectReports,
                     );
+                scope.clear_js_runtime();
             })
-            .expect("Thread spawning failed");
+            .expect("Thread spawning failed")
     }
 
     pub fn image_cache(&self) -> Arc<dyn ImageCache> {
@@ -485,7 +512,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 +532,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

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;
@@ -112,15 +115,46 @@ 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<AtomicBool>);
+pub struct AutoCloseWorker {
+    /// https://html.spec.whatwg.org/multipage/#dom-workerglobalscope-closing
+    closing: Arc<AtomicBool>,
+    /// A handle to join on the worker thread.
+    join_handle: Option<JoinHandle<()>>,
+    /// A sender of control messages,
+    /// currently only used to signal shutdown.
+    control_sender: Sender<DedicatedWorkerControlMsg>,
+}
 
 impl Drop for AutoCloseWorker {
+    /// <https://html.spec.whatwg.org/multipage/#terminate-a-worker>
     fn drop(&mut self) {
-        self.0.store(true, Ordering::SeqCst);
+        // 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.
+        if self
+            .join_handle
+            .take()
+            .expect("No handle to join on worker.")
+            .join()
+            .is_err()
+        {
+            warn!("Failed to join on dedicated worker thread.");
+        }
     }
 }
 
@@ -760,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,
@@ -1794,10 +1837,19 @@ impl GlobalScope {
         &self.permission_state_invocation_results
     }
 
-    pub fn track_worker(&self, closing_worker: Arc<AtomicBool>) {
+    pub fn track_worker(
+        &self,
+        closing: Arc<AtomicBool>,
+        join_handle: JoinHandle<()>,
+        control_sender: Sender<DedicatedWorkerControlMsg>,
+    ) {
         self.list_auto_close_worker
             .borrow_mut()
-            .push(AutoCloseWorker(closing_worker));
+            .push(AutoCloseWorker {
+                closing,
+                join_handle: Some(join_handle),
+                control_sender: control_sender,
+            });
     }
 
     pub fn track_event_source(&self, event_source: &EventSource) {

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};
 
@@ -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<ServiceWorkerMsg>,
 
     scope_url: ServoUrl,
+
+    /// A receiver of control messages,
+    /// currently only used to signal shutdown.
+    #[ignore_malloc_size_of = "Channels are hard"]
+    control_receiver: Receiver<ServiceWorkerControlMsg>,
 }
 
 impl WorkerEventLoopMethods for ServiceWorkerGlobalScope {
     type WorkerMsg = ServiceWorkerScriptMsg;
+    type ControlMsg = ServiceWorkerControlMsg;
     type Event = MixedMessage;
 
     fn task_queue(&self) -> &TaskQueue<ServiceWorkerScriptMsg> {
         &self.task_queue
     }
 
-    fn handle_event(&self, event: MixedMessage) {
+    fn handle_event(&self, event: MixedMessage) -> bool {
-        self.handle_mixed_message(event);
+        self.handle_mixed_message(event)
     }
 
     fn handle_worker_post_event(&self, _worker: &TrustedWorkerAddress) -> Option<AutoWorkerReset> {
         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<ServiceWorkerControlMsg> {
+        &self.control_receiver
+    }
 }
 
 impl ServiceWorkerGlobalScope {
@@ -203,6 +224,7 @@ impl ServiceWorkerGlobalScope {
         time_out_port: Receiver<Instant>,
         swmanager_sender: IpcSender<ServiceWorkerMsg>,
         scope_url: ServoUrl,
+        control_receiver: Receiver<ServiceWorkerControlMsg>,
     ) -> 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<Instant>,
         swmanager_sender: IpcSender<ServiceWorkerMsg>,
         scope_url: ServoUrl,
+        control_receiver: Receiver<ServiceWorkerControlMsg>,
     ) -> DomRoot<ServiceWorkerGlobalScope> {
         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,7 +284,8 @@ impl ServiceWorkerGlobalScope {
         devtools_receiver: IpcReceiver<DevtoolScriptControlMsg>,
         swmanager_sender: IpcSender<ServiceWorkerMsg>,
         scope_url: ServoUrl,
-    ) {
+        control_receiver: Receiver<ServiceWorkerControlMsg>,
+    ) -> JoinHandle<()> {
         let ScopeThings {
             script_url,
             init,
@@ -315,6 +341,7 @@ impl ServiceWorkerGlobalScope {
                     time_out_port,
                     swmanager_sender,
                     scope_url,
+                    control_receiver,
                 );
 
                 let (_url, source) =
@@ -361,35 +388,33 @@ impl ServiceWorkerGlobalScope {
                     );
                 scope.clear_js_runtime();
             })
-            .expect("Thread spawning failed");
+            .expect("Thread spawning failed")
     }
 
     fn handle_mixed_message(&self, msg: MixedMessage) -> bool {
         match msg {
-            MixedMessage::FromDevtools(msg) => {
+            MixedMessage::FromDevtools(msg) => match msg {
-                match msg {
+                DevtoolScriptControlMsg::EvaluateJS(_pipe_id, string, sender) => {
-                    DevtoolScriptControlMsg::EvaluateJS(_pipe_id, string, sender) => {
+                    devtools::handle_evaluate_js(self.upcast(), string, sender)
-                        devtools::handle_evaluate_js(self.upcast(), string, sender)
+                },
-                    },
+                DevtoolScriptControlMsg::WantsLiveNotifications(_pipe_id, bool_val) => {
-                    DevtoolScriptControlMsg::WantsLiveNotifications(_pipe_id, bool_val) => {
+                    devtools::handle_wants_live_notifications(self.upcast(), bool_val)
-                        devtools::handle_wants_live_notifications(self.upcast(), bool_val)
+                },
-                    },
+                _ => debug!("got an unusable devtools control message inside the worker!"),
-                    _ => debug!("got an unusable devtools control message inside the worker!"),
-                }
-                true
             },
             MixedMessage::FromServiceWorker(msg) => {
                 self.handle_script_event(msg);
-                true
+            },
+            MixedMessage::FromControl(ServiceWorkerControlMsg::Exit) => {
+                return false;
             },
         }
+        true
     }
 
     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
+        false
-        // immediately stop the event-loop after executing the initial script.
-        true
     }
 
     fn handle_script_event(&self, msg: ServiceWorkerScriptMsg) {

components/script/dom/window.rs

@@ -1443,8 +1443,8 @@ impl Window {
     }
 
     pub fn clear_js_runtime(&self) {
-        // Remove the infra for managing messageports and broadcast channels.
+        self.upcast::<GlobalScope>()
-        self.upcast::<GlobalScope>().remove_web_messaging_infra();
+            .remove_web_messaging_and_dedicated_workers_infra();
 
         // Clean up any active promises
         // https://github.com/servo/servo/issues/15318

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,9 @@ impl Worker {
 
         let init = prepare_workerscope_init(global, Some(devtools_sender), Some(worker_id));
 
-        DedicatedWorkerGlobalScope::run_worker_scope(
+        let (control_sender, control_receiver) = unbounded();
+
+        let join_handle = DedicatedWorkerGlobalScope::run_worker_scope(
             init,
             worker_url,
             devtools_receiver,
@@ -136,12 +137,15 @@ 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(),
+            control_receiver,
         );
 
+        global.track_worker(closing, join_handle, control_sender);
+
         Ok(worker)
     }
 

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::<GlobalScope>()
+            .remove_web_messaging_and_dedicated_workers_infra();
+
+        // Drop the runtime.
         let runtime = self.runtime.borrow_mut().take();
         drop(runtime);
     }

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};
@@ -24,7 +26,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 +79,33 @@ enum RegistrationUpdateTarget {
     Active,
 }
 
+impl Drop for ServiceWorkerRegistration {
+    /// <https://html.spec.whatwg.org/multipage/#terminate-a-worker>
+    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.
+        if self
+            .join_handle
+            .take()
+            .expect("No handle to join on worker.")
+            .join()
+            .is_err()
+        {
+            warn!("Failed to join on service worker thread.");
+        }
+    }
+}
+
 /// https://w3c.github.io/ServiceWorker/#service-worker-registration-concept
 struct ServiceWorkerRegistration {
     /// A unique identifer.
@@ -87,6 +116,11 @@ struct ServiceWorkerRegistration {
     waiting_worker: Option<ServiceWorker>,
     /// https://w3c.github.io/ServiceWorker/#dfn-installing-worker
     installing_worker: Option<ServiceWorker>,
+    /// A channel to send control message to the worker,
+    /// currently only used to signal shutdown.
+    control_sender: Option<Sender<ServiceWorkerControlMsg>>,
+    /// A handle to join on the worker thread.
+    join_handle: Option<JoinHandle<()>>,
 }
 
 impl ServiceWorkerRegistration {
@@ -96,9 +130,23 @@ impl ServiceWorkerRegistration {
             active_worker: None,
             waiting_worker: None,
             installing_worker: None,
+            join_handle: None,
+            control_sender: None,
         }
     }
 
+    fn note_worker_thread(
+        &mut self,
+        join_handle: JoinHandle<()>,
+        control_sender: Sender<ServiceWorkerControlMsg>,
+    ) {
+        assert!(self.join_handle.is_none());
+        self.join_handle = Some(join_handle);
+
+        assert!(self.control_sender.is_none());
+        self.control_sender = Some(control_sender);
+    }
+
     /// <https://w3c.github.io/ServiceWorker/#get-newest-worker>
     fn get_newest_worker(&self) -> Option<ServiceWorker> {
         if let Some(worker) = self.active_worker.as_ref() {
@@ -183,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;
             }
         }
@@ -326,9 +378,12 @@ impl ServiceWorkerManager {
 
             // Very roughly steps 5 to 18.
             // TODO: implement all steps precisely.
-            let new_worker =
+            let (new_worker, join_handle, control_sender) =
                 update_serviceworker(self.own_sender.clone(), job.scope_url.clone(), scope_things);
 
+            // 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.
 
             // Install: Step 4, run Update Registration State.
@@ -363,21 +418,32 @@ fn update_serviceworker(
     own_sender: IpcSender<ServiceWorkerMsg>,
     scope_url: ServoUrl,
     scope_things: ScopeThings,
-) -> ServiceWorker {
+) -> (
+    ServiceWorker,
+    JoinHandle<()>,
+    Sender<ServiceWorkerControlMsg>,
+) {
     let (sender, receiver) = unbounded();
     let (_devtools_sender, devtools_receiver) = ipc::channel().unwrap();
     let worker_id = ServiceWorkerId::new();
 
-    ServiceWorkerGlobalScope::run_serviceworker_scope(
+    let (control_sender, control_receiver) = unbounded();
+
+    let join_handle = ServiceWorkerGlobalScope::run_serviceworker_scope(
         scope_things.clone(),
         sender.clone(),
         receiver,
         devtools_receiver,
         own_sender,
         scope_url.clone(),
+        control_receiver,
     );
 
-    ServiceWorker::new(scope_things.script_url, sender, worker_id)
+    (
+        ServiceWorker::new(scope_things.script_url, sender, worker_id),
+        join_handle,
+        control_sender,
+    )
 }
 
 impl ServiceWorkerManagerFactory for ServiceWorkerManager {