From ed90d3a3654a80c1dc41d54ac8ed3fd85c51fdb6 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Sat, 23 Nov 2019 00:25:24 -0500 Subject: [PATCH 1/2] Ensure JS engine is shut down from same thread that initialized it. --- src/rust.rs | 53 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/src/rust.rs b/src/rust.rs index 25be18f15..57565700d 100644 --- a/src/rust.rs +++ b/src/rust.rs @@ -23,6 +23,7 @@ use std::os::raw::c_void; use std::cell::Cell; use std::marker::PhantomData; use std::sync::{Arc, Mutex}; +use std::sync::atomic::{AtomicU32, Ordering}; use consts::{JSCLASS_RESERVED_SLOTS_MASK, JSCLASS_GLOBAL_SLOT_COUNT}; use consts::{JSCLASS_IS_DOMJSCLASS, JSCLASS_IS_GLOBAL}; @@ -172,11 +173,31 @@ pub enum JSEngineError { /// A handle that must be kept alive in order to create new Runtimes. /// When this handle is dropped, the engine is shut down and cannot /// be reinitialized. -pub struct JSEngine(()); +pub struct JSEngine { + /// The count of alive handles derived from this initialized instance. + outstanding_handles: Arc, + // Ensure this type cannot be sent between threads. + marker: PhantomData<*mut ()>, +} + +pub struct JSEngineHandle(Arc); + +impl Clone for JSEngineHandle { + fn clone(&self) -> JSEngineHandle { + self.0.fetch_add(1, Ordering::SeqCst); + JSEngineHandle(self.0.clone()) + } +} + +impl Drop for JSEngineHandle { + fn drop(&mut self) { + self.0.fetch_sub(1, Ordering::SeqCst); + } +} impl JSEngine { /// Initialize the JS engine to prepare for creating new JS runtimes. - pub fn init() -> Result, JSEngineError> { + pub fn init() -> Result { let mut state = ENGINE_STATE.lock().unwrap(); match *state { EngineState::Initialized => return Err(JSEngineError::AlreadyInitialized), @@ -189,9 +210,22 @@ impl JSEngine { Err(JSEngineError::InitFailed) } else { *state = EngineState::Initialized; - Ok(Arc::new(JSEngine(()))) + Ok(JSEngine { + outstanding_handles: Arc::new(AtomicU32::new(0)), + marker: PhantomData, + }) } } + + pub fn can_shutdown(&self) -> bool { + self.outstanding_handles.load(Ordering::SeqCst) == 0 + } + + /// Create a handle to this engine. + pub fn handle(&self) -> JSEngineHandle { + self.outstanding_handles.fetch_add(1, Ordering::SeqCst); + JSEngineHandle(self.outstanding_handles.clone()) + } } /// Shut down the JS engine, invalidating any existing runtimes and preventing @@ -200,6 +234,11 @@ impl Drop for JSEngine { fn drop(&mut self) { let mut state = ENGINE_STATE.lock().unwrap(); if *state == EngineState::Initialized { + assert_eq!( + self.outstanding_handles.load(Ordering::SeqCst), + 0, + "There are outstanding JS engine handles" + ); *state = EngineState::ShutDown; unsafe { JS_ShutDown(); @@ -215,7 +254,7 @@ pub struct ParentRuntime { /// Raw pointer to the underlying SpiderMonkey runtime. parent: *mut JSRuntime, /// Handle to ensure the JS engine remains running while this handle exists. - engine: Arc, + engine: JSEngineHandle, /// The number of children of the runtime that created this ParentRuntime value. children_of_parent: Arc<()>, } @@ -226,7 +265,7 @@ pub struct Runtime { /// Raw pointer to the underlying SpiderMonkey context. cx: *mut JSContext, /// The engine that this runtime is associated with. - engine: Arc, + engine: JSEngineHandle, /// If this Runtime was created with a parent, this member exists to ensure /// that that parent's count of outstanding children (see [outstanding_children]) /// remains accurate and will be automatically decreased when this Runtime value @@ -251,7 +290,7 @@ impl Runtime { } /// Creates a new `JSContext`. - pub fn new(engine: Arc) -> Runtime { + pub fn new(engine: JSEngineHandle) -> Runtime { unsafe { Self::create(engine, None) } } @@ -279,7 +318,7 @@ impl Runtime { Self::create(parent.engine.clone(), Some(parent)) } - unsafe fn create(engine: Arc, parent: Option) -> Runtime { + unsafe fn create(engine: JSEngineHandle, parent: Option) -> Runtime { let parent_runtime = parent.as_ref().map_or( ptr::null_mut(), |r| r.parent, From c8c14531792ceb2d12c64a9a77dbfe087dcf8ae2 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Mon, 25 Nov 2019 15:36:05 -0500 Subject: [PATCH 2/2] Update unit tests. --- tests/callback.rs | 2 +- tests/capture_stack.rs | 2 +- tests/custom_auto_rooter.rs | 2 +- tests/custom_auto_rooter_macro.rs | 2 +- tests/enumerate.rs | 2 +- tests/evaluate.rs | 2 +- tests/panic.rs | 2 +- tests/rooting.rs | 2 +- tests/runtime.rs | 2 +- tests/runtime_no_outlive.rs | 2 +- tests/stack_limit.rs | 2 +- tests/typedarray.rs | 2 +- tests/typedarray_panic.rs | 2 +- tests/vec_conversion.rs | 2 +- 14 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/callback.rs b/tests/callback.rs index d6907fc43..7c8ab36ce 100644 --- a/tests/callback.rs +++ b/tests/callback.rs @@ -25,7 +25,7 @@ use std::str; #[test] fn callback() { let engine = JSEngine::init().unwrap(); - let runtime = Runtime::new(engine); + let runtime = Runtime::new(engine.handle()); let context = runtime.cx(); let h_option = OnNewGlobalHookOption::FireOnNewGlobalHook; let c_option = RealmOptions::default(); diff --git a/tests/capture_stack.rs b/tests/capture_stack.rs index 01cd92364..a6a65d873 100644 --- a/tests/capture_stack.rs +++ b/tests/capture_stack.rs @@ -22,7 +22,7 @@ use std::ptr; #[test] fn capture_stack() { let engine = JSEngine::init().unwrap(); - let runtime = Runtime::new(engine); + let runtime = Runtime::new(engine.handle()); let context = runtime.cx(); let h_option = OnNewGlobalHookOption::FireOnNewGlobalHook; let c_option = RealmOptions::default(); diff --git a/tests/custom_auto_rooter.rs b/tests/custom_auto_rooter.rs index dfdc2a82c..6a2632e90 100644 --- a/tests/custom_auto_rooter.rs +++ b/tests/custom_auto_rooter.rs @@ -34,7 +34,7 @@ unsafe impl CustomTrace for TraceCheck { #[test] fn virtual_trace_called() { let engine = JSEngine::init().unwrap(); - let rt = Runtime::new(engine); + let rt = Runtime::new(engine.handle()); let cx = rt.cx(); let mut rooter = CustomAutoRooter::new(TraceCheck::new()); diff --git a/tests/custom_auto_rooter_macro.rs b/tests/custom_auto_rooter_macro.rs index 1dff7cb62..1583b5879 100644 --- a/tests/custom_auto_rooter_macro.rs +++ b/tests/custom_auto_rooter_macro.rs @@ -31,7 +31,7 @@ unsafe impl CustomTrace for TraceCheck { #[test] fn custom_auto_rooter_macro() { let engine = JSEngine::init().unwrap(); - let rt = Runtime::new(engine); + let rt = Runtime::new(engine.handle()); let cx = rt.cx(); auto_root!(in(cx) let vec = vec![TraceCheck::new(), TraceCheck::new()]); diff --git a/tests/enumerate.rs b/tests/enumerate.rs index 7b8a5bb8b..0f8111c62 100644 --- a/tests/enumerate.rs +++ b/tests/enumerate.rs @@ -23,7 +23,7 @@ use std::ptr; #[test] fn enumerate() { let engine = JSEngine::init().unwrap(); - let rt = Runtime::new(engine); + let rt = Runtime::new(engine.handle()); let cx = rt.cx(); let options = RealmOptions::default(); diff --git a/tests/evaluate.rs b/tests/evaluate.rs index 0bcab2874..ae7f4a7ab 100644 --- a/tests/evaluate.rs +++ b/tests/evaluate.rs @@ -15,7 +15,7 @@ use std::ptr; #[test] fn evaluate() { let engine = JSEngine::init().unwrap(); - let rt = Runtime::new(engine); + let rt = Runtime::new(engine.handle()); let cx = rt.cx(); unsafe { diff --git a/tests/panic.rs b/tests/panic.rs index 61fdaf602..6d8c60715 100644 --- a/tests/panic.rs +++ b/tests/panic.rs @@ -20,7 +20,7 @@ use std::ptr; #[should_panic] fn test_panic() { let engine = JSEngine::init().unwrap(); - let runtime = Runtime::new(engine); + let runtime = Runtime::new(engine.handle()); let context = runtime.cx(); let h_option = OnNewGlobalHookOption::FireOnNewGlobalHook; let c_option = RealmOptions::default(); diff --git a/tests/rooting.rs b/tests/rooting.rs index 73dad2cee..ac65c0660 100644 --- a/tests/rooting.rs +++ b/tests/rooting.rs @@ -29,7 +29,7 @@ use std::ptr; fn rooting() { unsafe { let engine = JSEngine::init().unwrap(); - let runtime = Runtime::new(engine); + let runtime = Runtime::new(engine.handle()); let cx = runtime.cx(); JS_SetGCZeal(cx, 2, 1); diff --git a/tests/runtime.rs b/tests/runtime.rs index 641063836..8220b0c8e 100644 --- a/tests/runtime.rs +++ b/tests/runtime.rs @@ -24,7 +24,7 @@ use std::sync::mpsc::channel; fn runtime() { let engine = JSEngine::init().unwrap(); assert!(JSEngine::init().is_err()); - let runtime = Runtime::new(engine); + let runtime = Runtime::new(engine.handle()); unsafe { let cx = runtime.cx(); let h_option = OnNewGlobalHookOption::FireOnNewGlobalHook; diff --git a/tests/runtime_no_outlive.rs b/tests/runtime_no_outlive.rs index f6dca3e7d..1e3946aec 100644 --- a/tests/runtime_no_outlive.rs +++ b/tests/runtime_no_outlive.rs @@ -10,7 +10,7 @@ use mozjs::rust::{JSEngine, Runtime}; #[should_panic] fn runtime() { let engine = JSEngine::init().unwrap(); - let runtime = Runtime::new(engine); + let runtime = Runtime::new(engine.handle()); let _parent = runtime.prepare_for_new_child(); drop(runtime); } diff --git a/tests/stack_limit.rs b/tests/stack_limit.rs index 7bfaf03ac..8509dd348 100644 --- a/tests/stack_limit.rs +++ b/tests/stack_limit.rs @@ -15,7 +15,7 @@ use std::ptr; #[test] fn stack_limit() { let engine = JSEngine::init().unwrap(); - let rt = Runtime::new(engine); + let rt = Runtime::new(engine.handle()); let cx = rt.cx(); unsafe { diff --git a/tests/typedarray.rs b/tests/typedarray.rs index 9c1e52212..0dac4ba44 100644 --- a/tests/typedarray.rs +++ b/tests/typedarray.rs @@ -21,7 +21,7 @@ use std::ptr; #[test] fn typedarray() { let engine = JSEngine::init().unwrap(); - let rt = Runtime::new(engine); + let rt = Runtime::new(engine.handle()); let cx = rt.cx(); unsafe { diff --git a/tests/typedarray_panic.rs b/tests/typedarray_panic.rs index 4d84cfb1f..803b7a425 100644 --- a/tests/typedarray_panic.rs +++ b/tests/typedarray_panic.rs @@ -20,7 +20,7 @@ use std::ptr; #[should_panic] fn typedarray_update_panic() { let engine = JSEngine::init().unwrap(); - let rt = Runtime::new(engine); + let rt = Runtime::new(engine.handle()); let cx = rt.cx(); let options = RealmOptions::default(); diff --git a/tests/vec_conversion.rs b/tests/vec_conversion.rs index 081eb26f3..92f07337b 100644 --- a/tests/vec_conversion.rs +++ b/tests/vec_conversion.rs @@ -21,7 +21,7 @@ use std::ptr; #[test] fn vec_conversion() { let engine = JSEngine::init().unwrap(); - let rt = Runtime::new(engine); + let rt = Runtime::new(engine.handle()); let cx = rt.cx(); let h_option = OnNewGlobalHookOption::FireOnNewGlobalHook;