diff --git a/src/global.rs b/src/global.rs
index a1531836..80bfed61 100644
--- a/src/global.rs
+++ b/src/global.rs
@@ -3,6 +3,7 @@ use std::ptr::NonNull;
use crate::InIsolate;
use crate::Isolate;
+use crate::IsolateHandle;
use crate::Local;
use crate::ToLocal;
use crate::Value;
@@ -34,17 +35,15 @@ extern "C" {
#[repr(C)]
pub struct Global<T> {
value: Option<NonNull<T>>,
- isolate: Option<NonNull<Isolate>>,
+ isolate_handle: Option<IsolateHandle>,
}
-unsafe impl<T> Send for Global<T> {}
-
impl<T> Global<T> {
/// Construct a Global with no storage cell.
pub fn new() -> Self {
Self {
value: None,
- isolate: None,
+ isolate_handle: None,
}
}
@@ -60,7 +59,7 @@ impl<T> Global<T> {
Self {
value: other_value
.map(|v| unsafe { transmute(v8__Global__New(isolate, transmute(v))) }),
- isolate: other_value.map(|_| isolate.into()),
+ isolate_handle: other_value.map(|_| IsolateHandle::new(isolate)),
}
}
@@ -105,7 +104,7 @@ impl<T> Global<T> {
)
},
}
- self.isolate = other_value.map(|_| isolate.into());
+ self.isolate_handle = other_value.map(|_| IsolateHandle::new(isolate));
}
/// If non-empty, destroy the underlying storage cell
@@ -114,10 +113,13 @@ impl<T> Global<T> {
self.set(scope, None);
}
- fn check_isolate(&self, other: &Isolate) {
+ fn check_isolate(&self, isolate: &mut Isolate) {
match self.value {
- None => assert_eq!(self.isolate, None),
- Some(_) => assert_eq!(self.isolate.unwrap(), other.into()),
+ None => assert!(self.isolate_handle.is_none()),
+ Some(_) => assert_eq!(
+ unsafe { self.isolate_handle.as_ref().unwrap().get_isolate_ptr() },
+ isolate as *mut _
+ ),
}
}
}
@@ -130,30 +132,52 @@ impl<T> Default for Global<T> {
impl<T> Drop for Global<T> {
fn drop(&mut self) {
- if !self.is_empty() {
- panic!("Global handle dropped while holding a value");
+ match &mut self.value {
+ None => {
+ // This global handle is empty.
+ assert!(self.isolate_handle.is_none())
+ }
+ Some(_)
+ if unsafe {
+ self
+ .isolate_handle
+ .as_ref()
+ .unwrap()
+ .get_isolate_ptr()
+ .is_null()
+ } =>
+ {
+ // This global handle is associated with an Isolate that has already
+ // been disposed.
+ }
+ addr @ Some(_) => unsafe {
+ // Destroy the storage cell that contains the contents of this Global.
+ v8__Global__Reset__0(
+ &mut *(addr as *mut Option<NonNull<T>> as *mut *mut Value),
+ )
+ },
}
}
}
pub trait AnyHandle<T> {
- fn read(self, isolate: &Isolate) -> Option<NonNull<T>>;
+ fn read(self, isolate: &mut Isolate) -> Option<NonNull<T>>;
}
impl<'sc, T> AnyHandle<T> for Local<'sc, T> {
- fn read(self, _isolate: &Isolate) -> Option<NonNull<T>> {
+ fn read(self, _isolate: &mut Isolate) -> Option<NonNull<T>> {
Some(self.as_non_null())
}
}
impl<'sc, T> AnyHandle<T> for Option<Local<'sc, T>> {
- fn read(self, _isolate: &Isolate) -> Option<NonNull<T>> {
+ fn read(self, _isolate: &mut Isolate) -> Option<NonNull<T>> {
self.map(|local| local.as_non_null())
}
}
impl<'sc, T> AnyHandle<T> for &Global<T> {
- fn read(self, isolate: &Isolate) -> Option<NonNull<T>> {
+ fn read(self, isolate: &mut Isolate) -> Option<NonNull<T>> {
self.check_isolate(isolate);
self.value
}
diff --git a/src/isolate.rs b/src/isolate.rs
index 0bf0bab2..a922f00b 100644
--- a/src/isolate.rs
+++ b/src/isolate.rs
@@ -18,10 +18,15 @@ use crate::ScriptOrModule;
use crate::StartupData;
use crate::String;
use crate::Value;
+
use std::ffi::c_void;
+use std::mem::replace;
use std::ops::Deref;
use std::ops::DerefMut;
+use std::ptr::null_mut;
use std::ptr::NonNull;
+use std::sync::Arc;
+use std::sync::Mutex;
pub type MessageCallback = extern "C" fn(Local<Message>, Local<Value>);
@@ -161,22 +166,26 @@ impl Isolate {
CreateParams::new()
}
+ pub fn thread_safe_handle(&mut self) -> IsolateHandle {
+ IsolateHandle::new(self)
+ }
+
/// Associate embedder-specific data with the isolate. |slot| has to be
/// between 0 and GetNumberOfDataSlots() - 1.
pub unsafe fn set_data(&mut self, slot: u32, ptr: *mut c_void) {
- v8__Isolate__SetData(self, slot, ptr)
+ v8__Isolate__SetData(self, slot + 1, ptr)
}
/// Retrieve embedder-specific data from the isolate.
/// Returns NULL if SetData has never been called for the given |slot|.
pub fn get_data(&self, slot: u32) -> *mut c_void {
- unsafe { v8__Isolate__GetData(self, slot) }
+ unsafe { v8__Isolate__GetData(self, slot + 1) }
}
/// Returns the maximum number of available embedder data slots. Valid slots
/// are in the range of 0 - GetNumberOfDataSlots() - 1.
pub fn get_number_of_data_slots(&self) -> u32 {
- unsafe { v8__Isolate__GetNumberOfDataSlots(self) }
+ unsafe { v8__Isolate__GetNumberOfDataSlots(self) - 1 }
}
/// Sets this isolate as the entered one for the current thread.
@@ -283,23 +292,97 @@ impl Isolate {
}
}
+ /// Runs the default MicrotaskQueue until it gets empty.
+ /// Any exceptions thrown by microtask callbacks are swallowed.
+ pub fn run_microtasks(&mut self) {
+ unsafe { v8__Isolate__RunMicrotasks(self) }
+ }
+
+ /// Enqueues the callback to the default MicrotaskQueue
+ pub fn enqueue_microtask(&mut self, microtask: Local<Function>) {
+ unsafe { v8__Isolate__EnqueueMicrotask(self, microtask) }
+ }
+
+ /// Disposes the isolate. The isolate must not be entered by any
+ /// thread to be disposable.
+ unsafe fn dispose(&mut self) {
+ IsolateHandle::dispose(self);
+ v8__Isolate__Dispose(self)
+ }
+}
+
+pub(crate) struct IsolateAnnex {
+ isolate: *mut Isolate,
+ mutex: Mutex<()>,
+}
+
+impl IsolateAnnex {
+ fn new(isolate: &mut Isolate) -> Self {
+ Self {
+ isolate,
+ mutex: Mutex::new(()),
+ }
+ }
+}
+
+#[derive(Clone)]
+pub struct IsolateHandle(Arc<IsolateAnnex>);
+
+unsafe impl Send for IsolateHandle {}
+unsafe impl Sync for IsolateHandle {}
+
+impl IsolateHandle {
+ // This function is marked unsafe because it must be called only with either
+ // IsolateAnnex::mutex locked, or from the main thread associated with the V8
+ // isolate.
+ pub(crate) unsafe fn get_isolate_ptr(&self) -> *mut Isolate {
+ self.0.isolate
+ }
+
+ fn dispose(isolate: &mut Isolate) {
+ let annex_ptr = isolate.get_data(0) as *mut IsolateAnnex;
+ if !annex_ptr.is_null() {
+ unsafe {
+ isolate.set_data(0, null_mut());
+ let _lock = (*annex_ptr).mutex.lock().unwrap();
+ let isolate_ptr = replace(&mut (*annex_ptr).isolate, null_mut());
+ assert_eq!(isolate as *mut _, isolate_ptr);
+ Arc::from_raw(annex_ptr);
+ };
+ }
+ }
+
+ pub(crate) fn new(isolate: &mut Isolate) -> Self {
+ let annex_ptr = isolate.get_data(0) as *mut IsolateAnnex;
+ if annex_ptr.is_null() {
+ let annex_arc = Arc::new(IsolateAnnex::new(isolate));
+ let annex_ptr = Arc::into_raw(annex_arc.clone());
+ unsafe {
+ isolate.set_data(0, annex_ptr as *mut c_void);
+ }
+ IsolateHandle(annex_arc)
+ } else {
+ let annex_arc = unsafe { Arc::from_raw(annex_ptr) };
+ Arc::into_raw(annex_arc.clone());
+ IsolateHandle(annex_arc)
+ }
+ }
+
/// Forcefully terminate the current thread of JavaScript execution
/// in the given isolate.
///
/// This method can be used by any thread even if that thread has not
/// acquired the V8 lock with a Locker object.
- pub fn terminate_execution(&self) {
- unsafe { v8__Isolate__TerminateExecution(self) }
- }
-
- /// Is V8 terminating JavaScript execution.
///
- /// Returns true if JavaScript execution is currently terminating
- /// because of a call to TerminateExecution. In that case there are
- /// still JavaScript frames on the stack and the termination
- /// exception is still active.
- pub fn is_execution_terminating(&self) -> bool {
- unsafe { v8__Isolate__IsExecutionTerminating(self) }
+ /// Returns false if Isolate was already destroyed.
+ pub fn terminate_execution(&self) -> bool {
+ let _lock = self.0.mutex.lock().unwrap();
+ if self.0.isolate.is_null() {
+ false
+ } else {
+ unsafe { v8__Isolate__TerminateExecution(self.0.isolate) };
+ true
+ }
}
/// Resume execution capability in the given isolate, whose execution
@@ -314,19 +397,33 @@ impl Isolate {
///
/// This method can be used by any thread even if that thread has not
/// acquired the V8 lock with a Locker object.
- pub fn cancel_terminate_execution(&self) {
- unsafe { v8__Isolate__CancelTerminateExecution(self) }
+ ///
+ /// Returns false if Isolate was already destroyed.
+ pub fn cancel_terminate_execution(&self) -> bool {
+ let _lock = self.0.mutex.lock().unwrap();
+ if self.0.isolate.is_null() {
+ false
+ } else {
+ unsafe { v8__Isolate__CancelTerminateExecution(self.0.isolate) };
+ true
+ }
}
- /// Runs the default MicrotaskQueue until it gets empty.
- /// Any exceptions thrown by microtask callbacks are swallowed.
- pub fn run_microtasks(&mut self) {
- unsafe { v8__Isolate__RunMicrotasks(self) }
- }
-
- /// Enqueues the callback to the default MicrotaskQueue
- pub fn enqueue_microtask(&mut self, microtask: Local<Function>) {
- unsafe { v8__Isolate__EnqueueMicrotask(self, microtask) }
+ /// Is V8 terminating JavaScript execution.
+ ///
+ /// Returns true if JavaScript execution is currently terminating
+ /// because of a call to TerminateExecution. In that case there are
+ /// still JavaScript frames on the stack and the termination
+ /// exception is still active.
+ ///
+ /// Returns false if Isolate was already destroyed.
+ pub fn is_execution_terminating(&self) -> bool {
+ let _lock = self.0.mutex.lock().unwrap();
+ if self.0.isolate.is_null() {
+ false
+ } else {
+ unsafe { v8__Isolate__IsExecutionTerminating(self.0.isolate) }
+ }
}
/// Request V8 to interrupt long running JavaScript code and invoke
@@ -335,6 +432,8 @@ impl Isolate {
/// There may be a number of interrupt requests in flight.
/// Can be called from another thread without acquiring a |Locker|.
/// Registered |callback| must not reenter interrupted Isolate.
+ ///
+ /// Returns false if Isolate was already destroyed.
// Clippy warns that this method is dereferencing a raw pointer, but it is
// not: https://github.com/rust-lang/rust-clippy/issues/3045
#[allow(clippy::not_unsafe_ptr_arg_deref)]
@@ -342,19 +441,21 @@ impl Isolate {
&self,
callback: InterruptCallback,
data: *mut c_void,
- ) {
- unsafe { v8__Isolate__RequestInterrupt(self, callback, data) }
- }
-
- /// Disposes the isolate. The isolate must not be entered by any
- /// thread to be disposable.
- pub unsafe fn dispose(&mut self) {
- v8__Isolate__Dispose(self)
+ ) -> bool {
+ let _lock = self.0.mutex.lock().unwrap();
+ if self.0.isolate.is_null() {
+ false
+ } else {
+ unsafe { v8__Isolate__RequestInterrupt(self.0.isolate, callback, data) };
+ true
+ }
}
}
/// Internal method for constructing an OwnedIsolate.
-pub unsafe fn new_owned_isolate(isolate_ptr: *mut Isolate) -> OwnedIsolate {
+pub(crate) unsafe fn new_owned_isolate(
+ isolate_ptr: *mut Isolate,
+) -> OwnedIsolate {
OwnedIsolate(NonNull::new(isolate_ptr).unwrap())
}
diff --git a/src/lib.rs b/src/lib.rs
index 5bcb51fc..6b92f401 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -132,6 +132,7 @@ pub use isolate::CreateParams;
pub use isolate::HostImportModuleDynamicallyCallback;
pub use isolate::HostInitializeImportMetaObjectCallback;
pub use isolate::Isolate;
+pub use isolate::IsolateHandle;
pub use isolate::MessageCallback;
pub use isolate::OwnedIsolate;
pub use isolate::PromiseRejectCallback;
diff --git a/tests/test_api.rs b/tests/test_api.rs
index acbbbd7d..be8ece4f 100644
--- a/tests/test_api.rs
+++ b/tests/test_api.rs
@@ -126,6 +126,28 @@ fn global_handles() {
assert!(g5.is_empty());
}
+#[test]
+fn global_handle_drop() {
+ let _setup_guard = setup();
+
+ // Global 'g1' will be dropped _after_ the Isolate has been disposed.
+ let mut g1 = v8::Global::<v8::String>::new();
+
+ let mut params = v8::Isolate::create_params();
+ params.set_array_buffer_allocator(v8::new_default_allocator());
+ let mut isolate = v8::Isolate::new(params);
+
+ let mut hs = v8::HandleScope::new(&mut isolate);
+ let scope = hs.enter();
+
+ let l1 = v8::String::new(scope, "foo").unwrap();
+ g1.set(scope, l1);
+
+ // Global 'g2' will be dropped _before_ the Isolate has been disposed.
+ let l2 = v8::String::new(scope, "bar").unwrap();
+ let _g2 = v8::Global::new_from(scope, l2);
+}
+
#[test]
fn test_string() {
let _setup_guard = setup();
@@ -451,12 +473,47 @@ fn throw_exception() {
}
}
+#[test]
+fn thread_safe_handle_drop_after_isolate() {
+ let _setup_guard = setup();
+ let mut params = v8::Isolate::create_params();
+ params.set_array_buffer_allocator(v8::new_default_allocator());
+ let mut isolate = v8::Isolate::new(params);
+ let handle = isolate.thread_safe_handle();
+ // We can call it twice.
+ let handle_ = isolate.thread_safe_handle();
+ // Check that handle is Send and Sync.
+ fn f<S: Send + Sync>(_: S) {}
+ f(handle_);
+ // All methods on IsolateHandle should return false after the isolate is
+ // dropped.
+ drop(isolate);
+ assert_eq!(false, handle.terminate_execution());
+ assert_eq!(false, handle.cancel_terminate_execution());
+ assert_eq!(false, handle.is_execution_terminating());
+ static CALL_COUNT: AtomicUsize = AtomicUsize::new(0);
+ extern "C" fn callback(
+ _isolate: &mut v8::Isolate,
+ data: *mut std::ffi::c_void,
+ ) {
+ assert_eq!(data, std::ptr::null_mut());
+ CALL_COUNT.fetch_add(1, Ordering::SeqCst);
+ }
+ assert_eq!(
+ false,
+ handle.request_interrupt(callback, std::ptr::null_mut())
+ );
+ assert_eq!(CALL_COUNT.load(Ordering::SeqCst), 0);
+}
+
+// TODO(ry) This test should use threads
#[test]
fn terminate_execution() {
let _setup_guard = setup();
let mut params = v8::Isolate::create_params();
params.set_array_buffer_allocator(v8::new_default_allocator());
let mut isolate = v8::Isolate::new(params);
+ let handle = isolate.thread_safe_handle();
// Originally run fine.
{
let mut hs = v8::HandleScope::new(&mut isolate);
@@ -469,7 +526,7 @@ fn terminate_execution() {
assert!(result.same_value(true_val));
}
// Terminate.
- isolate.terminate_execution();
+ handle.terminate_execution();
// Below run should fail with terminated knowledge.
{
let mut hs = v8::HandleScope::new(&mut isolate);
@@ -484,7 +541,7 @@ fn terminate_execution() {
assert!(tc.has_terminated());
}
// Cancel termination.
- isolate.cancel_terminate_execution();
+ handle.cancel_terminate_execution();
// Works again.
{
let mut hs = v8::HandleScope::new(&mut isolate);
@@ -498,12 +555,14 @@ fn terminate_execution() {
}
}
+// TODO(ry) This test should use threads
#[test]
fn request_interrupt_small_scripts() {
let _setup_guard = setup();
let mut params = v8::Isolate::create_params();
params.set_array_buffer_allocator(v8::new_default_allocator());
let mut isolate = v8::Isolate::new(params);
+ let handle = isolate.thread_safe_handle();
{
let mut hs = v8::HandleScope::new(&mut isolate);
let scope = hs.enter();
@@ -519,9 +578,7 @@ fn request_interrupt_small_scripts() {
assert_eq!(data, std::ptr::null_mut());
CALL_COUNT.fetch_add(1, Ordering::SeqCst);
}
- scope
- .isolate()
- .request_interrupt(callback, std::ptr::null_mut());
+ handle.request_interrupt(callback, std::ptr::null_mut());
eval(scope, context, "(function(x){return x;})(1);");
assert_eq!(CALL_COUNT.load(Ordering::SeqCst), 1);
}