diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index c9e75476974..d0978dfed84 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -461,6 +461,9 @@ pub struct Constellation { /// The async runtime. async_runtime: Box, + + /// When in single-process mode, join handles for script-threads. + script_join_handles: HashMap>, } /// State needed to construct a constellation. @@ -711,6 +714,7 @@ where user_content_manager: state.user_content_manager, process_manager: ProcessManager::new(state.mem_profiler_chan), async_runtime: state.async_runtime, + script_join_handles: Default::default(), }; constellation.run(); @@ -972,6 +976,10 @@ where self.background_monitor_control_senders.push(chan); } + if let Some(join_handle) = pipeline.join_handle { + self.script_join_handles.insert(webview_id, join_handle); + } + if let Some(host) = host { debug!("{}: Adding new host entry {}", webview_id, host,); self.set_event_loop( @@ -2523,12 +2531,20 @@ where fn handle_shutdown(&mut self) { debug!("Handling shutdown."); + // In single process mode, join on script-threads + // from webview which haven't been manually closed before. + for (_, join_handle) in self.script_join_handles.drain() { + if join_handle.join().is_err() { + error!("Failed to join on a script-thread."); + } + } + // In single process mode, join on the background hang monitor worker thread. drop(self.background_monitor_register.take()); if let Some(join_handle) = self.background_monitor_register_join_handle.take() { - join_handle - .join() - .expect("Failed to join on the BHM background thread."); + if join_handle.join().is_err() { + error!("Failed to join on the bhm background thread."); + } } // At this point, there are no active pipelines, @@ -3053,6 +3069,11 @@ where .remove(&browsing_context.bc_group_id); } + // Note: In single-process mode, + // if the webview is manually closed, we drop the join handle without joining on it. + // It is unlikely the thread will still run when the constellation shuts-down. + self.script_join_handles.remove(&webview_id); + debug!("{webview_id}: Closed"); } diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index 6f4923000c2..cf7c7826bcb 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -209,6 +209,7 @@ pub struct NewPipeline { pub pipeline: Pipeline, pub bhm_control_chan: Option>, pub lifeline: Option<(IpcReceiver<()>, Process)>, + pub join_handle: Option>, } impl Pipeline { @@ -218,7 +219,7 @@ impl Pipeline { ) -> Result { // Note: we allow channel creation to panic, since recovering from this // probably requires a general low-memory strategy. - let (script_chan, (bhm_control_chan, lifeline)) = match state.event_loop { + let (script_chan, (bhm_control_chan, lifeline, join_handle)) = match state.event_loop { Some(script_chan) => { let new_layout_info = NewLayoutInfo { parent_info: state.parent_pipeline_id, @@ -235,7 +236,7 @@ impl Pipeline { { warn!("Sending to script during pipeline creation failed ({})", e); } - (script_chan, (None, None)) + (script_chan, (None, None, None)) }, None => { let (script_chan, script_port) = ipc::channel().expect("Pipeline script chan"); @@ -315,18 +316,18 @@ impl Pipeline { ipc::channel().expect("Failed to create lifeline channel"); unprivileged_pipeline_content.lifeline_sender = Some(sender); let process = unprivileged_pipeline_content.spawn_multiprocess()?; - (Some(bhm_control_chan), Some((receiver, process))) + (Some(bhm_control_chan), Some((receiver, process)), None) } else { // Should not be None in single-process mode. let register = state .background_monitor_register .expect("Couldn't start content, no background monitor has been initiated"); - unprivileged_pipeline_content.start_all::( + let join_handle = unprivileged_pipeline_content.start_all::( false, state.layout_factory, register, ); - (None, None) + (None, None, Some(join_handle)) }; (EventLoop::new(script_chan), multiprocess_data) @@ -347,6 +348,7 @@ impl Pipeline { pipeline, bhm_control_chan, lifeline, + join_handle, }) } @@ -501,7 +503,7 @@ impl UnprivilegedPipelineContent { wait_for_completion: bool, layout_factory: Arc, background_hang_monitor_register: Box, - ) { + ) -> JoinHandle<()> { // Setup pipeline-namespace-installing for all threads in this process. // Idempotent in single-process mode. PipelineNamespace::set_installer_sender(self.namespace_request_sender); @@ -511,7 +513,7 @@ impl UnprivilegedPipelineContent { self.rippy_data, )); let (content_process_shutdown_chan, content_process_shutdown_port) = unbounded(); - STF::create( + let join_handle = STF::create( InitialScriptState { id: self.id, browsing_context_id: self.browsing_context_id, @@ -551,6 +553,8 @@ impl UnprivilegedPipelineContent { Err(_) => error!("Script-thread shut-down unexpectedly"), } } + + join_handle } pub fn spawn_multiprocess(self) -> Result { diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index a5e077347a2..6e1766ef301 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -25,7 +25,7 @@ use std::rc::Rc; use std::result::Result; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; -use std::thread; +use std::thread::{self, JoinHandle}; use std::time::{Duration, Instant, SystemTime}; use background_hang_monitor_api::{ @@ -412,7 +412,7 @@ impl ScriptThreadFactory for ScriptThread { layout_factory: Arc, system_font_service: Arc, load_data: LoadData, - ) { + ) -> JoinHandle<()> { thread::Builder::new() .name(format!("Script{:?}", state.id)) .spawn(move || { @@ -462,7 +462,7 @@ impl ScriptThreadFactory for ScriptThread { // This must always be the very last operation performed before the thread completes failsafe.neuter(); }) - .expect("Thread spawning failed"); + .expect("Thread spawning failed") } } diff --git a/components/servo/lib.rs b/components/servo/lib.rs index 72ec27a4c5c..11ca8b2baa6 100644 --- a/components/servo/lib.rs +++ b/components/servo/lib.rs @@ -1265,7 +1265,7 @@ pub fn run_content_process(token: String) { content.register_system_memory_reporter(); - content.start_all::( + let script_join_handle = content.start_all::( true, layout_factory, background_hang_monitor_register, @@ -1274,7 +1274,10 @@ pub fn run_content_process(token: String) { // Since wait_for_completion is true, // here we know that the script-thread // will exit(or already has), - // and so we can join on the BHM worker thread. + // and so we can join first on the script, and then on the BHM worker, threads. + script_join_handle + .join() + .expect("Failed to join on the script thread."); join_handle .join() .expect("Failed to join on the BHM background thread."); diff --git a/components/shared/layout/lib.rs b/components/shared/layout/lib.rs index 0a228eaa8ee..f3efb640129 100644 --- a/components/shared/layout/lib.rs +++ b/components/shared/layout/lib.rs @@ -15,6 +15,7 @@ use std::any::Any; use std::collections::HashMap; use std::sync::Arc; use std::sync::atomic::{AtomicIsize, AtomicU64, Ordering}; +use std::thread::JoinHandle; use app_units::Au; use atomic_refcell::AtomicRefCell; @@ -305,7 +306,7 @@ pub trait ScriptThreadFactory { layout_factory: Arc, system_font_service: Arc, load_data: LoadData, - ); + ) -> JoinHandle<()>; } #[derive(Clone, Default)] pub struct OffsetParentResponse {