script: Initialize and shut down JS engine on the same thread.

Cargo.toml

@@ -29,3 +29,6 @@ opt-level = 3
 mio = { git = "https://github.com/servo/mio.git", branch = "servo" }
 # https://github.com/retep998/winapi-rs/pull/816
 winapi = { git = "https://github.com/servo/winapi-rs", branch = "patch-1" }
+
+[patch."https://github.com/servo/rust-mozjs"]
+mozjs = { git = "https://github.com/jdm/rust-mozjs", branch = "shutdown" }

components/constellation/constellation.rs

@@ -478,6 +478,9 @@ pub struct Constellation<Message, LTF, STF> {
 
     /// Pipeline ID of the active media session.
     active_media_session: Option<PipelineId>,
+
+    /// A list of senders to content process script shutdown threads.
+    script_shutdown_senders: Vec<IpcSender<()>>,
 }
 
 /// State needed to construct a constellation.
@@ -534,6 +537,9 @@ pub struct InitialConstellationState {
 
     /// Mechanism to force the compositor to process events.
     pub event_loop_waker: Option<Box<dyn EventLoopWaker>>,
+
+    /// A channel to the initial content process' script shutdown thread.
+    pub script_shutdown_sender: IpcSender<()>,
 }
 
 /// Data needed for webdriver
@@ -848,6 +854,7 @@ where
                     player_context: state.player_context,
                     event_loop_waker: state.event_loop_waker,
                     active_media_session: None,
+                    script_shutdown_senders: vec![state.script_shutdown_sender],
                 };
 
                 constellation.run();
@@ -1101,8 +1108,11 @@ where
             Err(e) => return self.handle_send_error(pipeline_id, e),
         };
 
-        if let Some(sampler_chan) = pipeline.sampler_control_chan {
-            self.sampling_profiler_control.push(sampler_chan);
+        if let Some(content_data) = pipeline.content_data {
+            self.sampling_profiler_control
+                .push(content_data.sampler_control_chan);
+            self.script_shutdown_senders
+                .push(content_data.script_shutdown_sender);
         }
 
         if let Some(host) = host {
@@ -2380,6 +2390,10 @@ where
             warn!("Exit storage thread failed ({})", e);
         }
 
+        for sender in &self.script_shutdown_senders {
+            let _ = sender.send(());
+        }
+
         debug!("Asking compositor to complete shutdown.");
         self.compositor_proxy
             .send(ToCompositorMsg::ShutdownComplete);

components/constellation/pipeline.rs

@@ -201,9 +201,14 @@ pub struct InitialPipelineState {
     pub event_loop_waker: Option<Box<dyn EventLoopWaker>>,
 }
 
+pub struct ContentData {
+    pub sampler_control_chan: IpcSender<SamplerControlMsg>,
+    pub script_shutdown_sender: IpcSender<()>,
+}
+
 pub struct NewPipeline {
     pub pipeline: Pipeline,
-    pub sampler_control_chan: Option<IpcSender<SamplerControlMsg>>,
+    pub content_data: Option<ContentData>,
 }
 
 impl Pipeline {
@@ -218,7 +223,7 @@ impl Pipeline {
         // probably requires a general low-memory strategy.
         let (pipeline_chan, pipeline_port) = ipc::channel().expect("Pipeline main chan");
 
-        let (script_chan, sampler_chan) = match state.event_loop {
+        let (script_chan, content_data) = match state.event_loop {
             Some(script_chan) => {
                 let new_layout_info = NewLayoutInfo {
                     parent_info: state.parent_pipeline_id,
@@ -301,16 +306,24 @@ impl Pipeline {
                     webvr_chan: state.webvr_chan,
                     webxr_registry: state.webxr_registry,
                     player_context: state.player_context,
+                    script_shutdown_receiver: None,
                 };
 
                 // Spawn the child process.
                 //
                 // Yes, that's all there is to it!
-                let sampler_chan = if opts::multiprocess() {
+                let content_data = if opts::multiprocess() {
                     let (sampler_chan, sampler_port) = ipc::channel().expect("Sampler chan");
                     unprivileged_pipeline_content.sampling_profiler_port = Some(sampler_port);
+                    let (script_shutdown_sender, script_shutdown_receiver) =
+                        ipc::channel().unwrap();
+                    unprivileged_pipeline_content.script_shutdown_receiver =
+                        Some(script_shutdown_receiver);
                     let _ = unprivileged_pipeline_content.spawn_multiprocess()?;
-                    Some(sampler_chan)
+                    Some(ContentData {
+                        sampler_control_chan: sampler_chan,
+                        script_shutdown_sender,
+                    })
                 } else {
                     // Should not be None in single-process mode.
                     let register = state
@@ -324,7 +337,7 @@ impl Pipeline {
                     None
                 };
 
-                (EventLoop::new(script_chan), sampler_chan)
+                (EventLoop::new(script_chan), content_data)
             },
         };
 
@@ -341,7 +354,7 @@ impl Pipeline {
         );
         Ok(NewPipeline {
             pipeline,
-            sampler_control_chan: sampler_chan,
+            content_data,
         })
     }
 
@@ -507,6 +520,7 @@ pub struct UnprivilegedPipelineContent {
     webvr_chan: Option<IpcSender<WebVRMsg>>,
     webxr_registry: webxr_api::Registry,
     player_context: WindowGLContext,
+    script_shutdown_receiver: Option<IpcReceiver<()>>,
 }
 
 impl UnprivilegedPipelineContent {
@@ -732,6 +746,12 @@ impl UnprivilegedPipelineContent {
         )
     }
 
+    pub fn script_shutdown_receiver(&mut self) -> IpcReceiver<()> {
+        self.script_shutdown_receiver
+            .take()
+            .expect("no shutdown receiver?")
+    }
+
     pub fn script_to_constellation_chan(&self) -> &ScriptToConstellationChan {
         &self.script_to_constellation_chan
     }

components/script/init.rs

@@ -4,7 +4,9 @@
 
 use crate::dom::bindings::codegen::RegisterBindings;
 use crate::dom::bindings::proxyhandler;
+use crate::script_runtime::setup_js_engine;
 use crate::serviceworker_manager::ServiceWorkerManager;
+use ipc_channel::ipc::{self, IpcReceiver, IpcSender};
 use script_traits::SWManagerSenders;
 
 #[cfg(target_os = "linux")]
@@ -55,8 +57,10 @@ pub fn init_service_workers(sw_senders: SWManagerSenders) {
     ServiceWorkerManager::spawn_manager(sw_senders);
 }
 
+/// Initialize the global script state, and listen for a shutdown signal
+/// on the provided receiver.
 #[allow(unsafe_code)]
-pub fn init() {
+pub fn init_with_shutdown_receiver(shutdown_receiver: IpcReceiver<()>) {
     unsafe {
         proxyhandler::init();
 
@@ -66,4 +70,14 @@ pub fn init() {
     }
 
     perform_platform_specific_initialization();
+
+    setup_js_engine(shutdown_receiver)
+}
+
+/// Initialize the global script state, and return a sender that will
+/// initiate shutdown procedures.
+pub fn init() -> IpcSender<()> {
+    let (shutdown_sender, shutdown_receiver) = ipc::channel().unwrap();
+    init_with_shutdown_receiver(shutdown_receiver);
+    shutdown_sender
 }

components/script/lib.rs

@@ -112,7 +112,7 @@ mod unpremultiplytable;
 #[warn(deprecated)]
 mod webdriver_handlers;
 
-pub use init::{init, init_service_workers};
+pub use init::{init, init_service_workers, init_with_shutdown_receiver};
 
 /// A module with everything layout can use from script.
 ///

components/script/script_runtime.rs

@@ -34,6 +34,9 @@ use crate::script_thread::trace_thread;
 use crate::task::TaskBox;
 use crate::task_source::networking::NetworkingTaskSource;
 use crate::task_source::{TaskSource, TaskSourceName};
+use crossbeam_channel::{select, unbounded, Sender};
+use ipc_channel::ipc::IpcReceiver;
+use ipc_channel::router::ROUTER;
 use js::glue::{CollectServoSizes, CreateJobQueue, DeleteJobQueue, DispatchableRun};
 use js::glue::{JobQueueTraps, RUST_js_GetErrorMessage, SetBuildId, StreamConsumerConsumeChunk};
 use js::glue::{
@@ -61,9 +64,9 @@ use js::rust::wrappers::{GetPromiseIsHandled, JS_GetPromiseResult};
 use js::rust::Handle;
 use js::rust::HandleObject as RustHandleObject;
 use js::rust::IntoHandle;
-use js::rust::JSEngine;
 use js::rust::ParentRuntime;
 use js::rust::Runtime as RustRuntime;
+use js::rust::{JSEngine, JSEngineHandle};
 use malloc_size_of::MallocSizeOfOps;
 use msg::constellation_msg::PipelineId;
 use profile_traits::mem::{Report, ReportKind, ReportsChan};
@@ -79,7 +82,9 @@ use std::os::raw::c_void;
 use std::panic::AssertUnwindSafe;
 use std::ptr;
 use std::rc::Rc;
-use std::sync::Arc;
+use std::sync::{Arc, Mutex};
+use std::thread;
+use std::time::Duration;
 use style::thread_state::{self, ThreadState};
 use time::{now, Tm};
 
@@ -396,7 +401,72 @@ impl Deref for Runtime {
 }
 
 lazy_static! {
-    static ref JS_ENGINE: Arc<JSEngine> = JSEngine::init().unwrap();
+    /// A single global channel for communicating with the long-lived JS engine thread
+    /// to obtain proof that the engine has been initialized and is running.
+    static ref JS_BOOTSTRAPPER: Arc<Mutex<Option<Sender<Sender<JSEngineHandle>>>>> =
+        Arc::new(Mutex::new(None));
+}
+
+pub(crate) fn setup_js_engine(ipc_shutdown_receiver: IpcReceiver<()>) {
+    let (shutdown_sender, shutdown_receiver) = unbounded();
+    ROUTER.route_ipc_receiver_to_crossbeam_sender(ipc_shutdown_receiver, shutdown_sender);
+
+    // This thread is long-lived, and exists only to initialize SpiderMonkey and shut
+    // it down in the same thread once all other users of the JS engine have been
+    // cleaned up.
+    thread::spawn(move || {
+        let js_engine = JSEngine::init().unwrap();
+
+        // Initialize the global channel so script threads can use it
+        // to communicate with this thread.
+        let (bootstrap_sender, bootstrap_receiver) = unbounded();
+        *JS_BOOTSTRAPPER.lock().unwrap() = Some(bootstrap_sender);
+
+        loop {
+            select! {
+                recv(shutdown_receiver) -> _msg => {
+                    // The browser is shutting down; time to
+                    // clean up the JS engine.
+                    break;
+                }
+                recv(bootstrap_receiver) -> msg => {
+                    // A script thread needs proof that the JS engine
+                    // is initialized in order to create a new runtime.
+                    if let Ok(sender) = msg {
+                        let _ = sender.send(js_engine.handle());
+                    }
+                }
+            }
+        }
+
+        // It is unsafe to shut down the JS engine while any other threads
+        // are still using it. The constellation closes all pipelines' script
+        // threads, but cannot wait for them to report completion without
+        // risking deadlocks. Under the assumption that given enough time all
+        // threads using the engine will either shut down or panic, we just
+        // need to outlive them.
+        while !js_engine.can_shutdown() {
+            thread::sleep(Duration::from_millis(10));
+        }
+
+        // We're done with the engine. Time to die.
+        drop(js_engine);
+    });
+}
+
+/// Acquire a token providing that the JS engine has been initialized.
+/// Requires performing a synchronous communication with the thread that
+/// initialized the JS engine.
+fn get_js_handle() -> JSEngineHandle {
+    let (tx, rx) = unbounded();
+    JS_BOOTSTRAPPER
+        .lock()
+        .unwrap()
+        .as_ref()
+        .expect("no JS bootstrap channel")
+        .send(tx)
+        .expect("JS bootstrap thread is missing");
+    rx.recv().expect("JS bootstrap thread response is missing")
 }
 
 #[allow(unsafe_code)]
@@ -421,7 +491,7 @@ unsafe fn new_rt_and_cx_with_parent(
     let runtime = if let Some(parent) = parent {
         RustRuntime::create_with_parent(parent)
     } else {
-        RustRuntime::new(JS_ENGINE.clone())
+        RustRuntime::new(get_js_handle())
     };
     let cx = runtime.cx();
 

components/servo/lib.rs

@@ -414,7 +414,7 @@ where
 
         // Important that this call is done in a single-threaded fashion, we
         // can't defer it after `create_constellation` has started.
-        script::init();
+        let script_shutdown_sender = script::init();
 
         if pref!(dom.webxr.enabled) && pref!(dom.webvr.enabled) {
             panic!("We don't currently support running both WebVR and WebXR");
@@ -516,6 +516,7 @@ where
             glplayer_threads,
             event_loop_waker,
             window_size,
+            script_shutdown_sender,
         );
 
         // Send the constellation's swmanager sender to service worker manager thread
@@ -840,6 +841,7 @@ fn create_constellation(
     glplayer_threads: Option<GLPlayerThreads>,
     event_loop_waker: Option<Box<dyn EventLoopWaker>>,
     initial_window_size: WindowSizeData,
+    script_shutdown_sender: IpcSender<()>,
 ) -> (Sender<ConstellationMsg>, SWManagerSenders) {
     // Global configuration options, parsed from the command line.
     let opts = opts::get();
@@ -882,6 +884,7 @@ fn create_constellation(
         glplayer_threads,
         player_context,
         event_loop_waker,
+        script_shutdown_sender,
     };
     let (constellation_chan, from_swmanager_sender) = Constellation::<
         script_layout_interface::message::Msg,
@@ -976,8 +979,8 @@ pub fn run_content_process(token: String) {
 
     // send the required channels to the service worker manager
     let sw_senders = unprivileged_content.swmanager_senders();
-    script::init();
     script::init_service_workers(sw_senders);
+    script::init_with_shutdown_receiver(unprivileged_content.script_shutdown_receiver());
 
     media_platform::init();