From ed688fe2c1fe004f6c4007bb08dc792a6ab6bcac Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Sun, 24 May 2020 15:39:57 +0800 Subject: [PATCH] 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 {