Skip to content

Commit

Permalink
Auto merge of #24862 - jdm:js-init-destroy-fit-the-second, r=asajeffrey
Browse files Browse the repository at this point in the history
Ensure SpiderMonkey shuts down cleanly

This is the alternate solution that I described in #24845. Given how much simpler the resulting code is, I'm now much more in favour of this design. Depends on servo/rust-mozjs#487.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #21696 and fix #22276
- [x] There are tests for these changes
  • Loading branch information
bors-servo committed Nov 27, 2019
2 parents b62d1d4 + 1c7f2d6 commit f166422
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 9 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion components/script/init.rs
Expand Up @@ -4,6 +4,7 @@

use crate::dom::bindings::codegen::RegisterBindings;
use crate::dom::bindings::proxyhandler;
use crate::script_runtime::JSEngineSetup;
use crate::serviceworker_manager::ServiceWorkerManager;
use script_traits::SWManagerSenders;

Expand Down Expand Up @@ -56,7 +57,7 @@ pub fn init_service_workers(sw_senders: SWManagerSenders) {
}

#[allow(unsafe_code)]
pub fn init() {
pub fn init() -> JSEngineSetup {
unsafe {
proxyhandler::init();

Expand All @@ -66,4 +67,6 @@ pub fn init() {
}

perform_platform_specific_initialization();

JSEngineSetup::new()
}
1 change: 1 addition & 0 deletions components/script/lib.rs
Expand Up @@ -113,6 +113,7 @@ mod unpremultiplytable;
mod webdriver_handlers;

pub use init::{init, init_service_workers};
pub use script_runtime::JSEngineSetup;

/// A module with everything layout can use from script.
///
Expand Down
32 changes: 27 additions & 5 deletions components/script/script_runtime.rs
Expand Up @@ -61,9 +61,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};
Expand All @@ -79,7 +79,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::Mutex;
use std::thread;
use std::time::Duration;
use style::thread_state::{self, ThreadState};
use time::{now, Tm};

Expand Down Expand Up @@ -395,8 +397,28 @@ impl Deref for Runtime {
}
}

pub struct JSEngineSetup(JSEngine);

impl JSEngineSetup {
pub fn new() -> Self {
let engine = JSEngine::init().unwrap();
*JS_ENGINE.lock().unwrap() = Some(engine.handle());
Self(engine)
}
}

impl Drop for JSEngineSetup {
fn drop(&mut self) {
*JS_ENGINE.lock().unwrap() = None;

while !self.0.can_shutdown() {
thread::sleep(Duration::from_millis(50));
}
}
}

lazy_static! {
static ref JS_ENGINE: Arc<JSEngine> = JSEngine::init().unwrap();
static ref JS_ENGINE: Mutex<Option<JSEngineHandle>> = Mutex::new(None);
}

#[allow(unsafe_code)]
Expand All @@ -421,7 +443,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(JS_ENGINE.lock().unwrap().as_ref().unwrap().clone())
};
let cx = runtime.cx();

Expand Down Expand Up @@ -794,7 +816,7 @@ unsafe fn set_gc_zeal_options(cx: *mut RawJSContext) {
use js::jsapi::{JS_SetGCZeal, JS_DEFAULT_ZEAL_FREQ};

let level = match pref!(js.mem.gc.zeal.level) {
level @ 0...14 => level as u8,
level @ 0..=14 => level as u8,
_ => return,
};
let frequency = match pref!(js.mem.gc.zeal.frequency) {
Expand Down
14 changes: 12 additions & 2 deletions components/servo/lib.rs
Expand Up @@ -105,6 +105,7 @@ use profile::mem as profile_mem;
use profile::time as profile_time;
use profile_traits::mem;
use profile_traits::time;
use script::JSEngineSetup;
use script_traits::{
ConstellationMsg, SWManagerSenders, ScriptToConstellationChan, WindowSizeData,
};
Expand Down Expand Up @@ -271,6 +272,10 @@ pub struct Servo<Window: WindowMethods + 'static + ?Sized> {
embedder_receiver: EmbedderReceiver,
embedder_events: Vec<(Option<BrowserId>, EmbedderMsg)>,
profiler_enabled: bool,
/// For single-process Servo instances, this field controls the initialization
/// and deinitialization of the JS Engine. Multiprocess Servo instances have their
/// own instance that exists in the content process instead.
_js_engine_setup: Option<JSEngineSetup>,
}

#[derive(Clone)]
Expand Down Expand Up @@ -415,7 +420,11 @@ 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 js_engine_setup = if !opts.multiprocess {
Some(script::init())
} else {
None
};

if pref!(dom.webxr.enabled) && pref!(dom.webvr.enabled) {
panic!("We don't currently support running both WebVR and WebXR");
Expand Down Expand Up @@ -556,6 +565,7 @@ where
embedder_receiver: embedder_receiver,
embedder_events: Vec::new(),
profiler_enabled: false,
_js_engine_setup: js_engine_setup,
}
}

Expand Down Expand Up @@ -980,7 +990,7 @@ 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();
let _js_engine_setup = script::init();
script::init_service_workers(sw_senders);

media_platform::init();
Expand Down

0 comments on commit f166422

Please sign in to comment.