Add new terminate_execution test (#288)

src/isolate.rs

@@ -235,7 +235,8 @@ impl Isolate {
     assert!(existing.is_none());
 
     // ptr is still stored in Isolate's embedder slot.
-    let _ptr = Box::into_raw(states);
+    let ptr2 = Box::into_raw(states);
+    assert_eq!(ptr, ptr2);
   }
 
   pub fn state_get<S>(&self) -> Rc<RefCell<S>>
@@ -251,7 +252,8 @@ impl Isolate {
     // Rc<RefCell<S>> without transmute?
     #[allow(clippy::transmute_ptr_to_ptr)]
     let state = unsafe { std::mem::transmute::<_, &Rc<RefCell<S>>>(state) };
-    let _ptr = Box::into_raw(states); // because isolate slot 0 still has ptr.
+    let ptr2 = Box::into_raw(states); // because isolate slot still has ptr.
+    assert_eq!(ptr, ptr2);
     state.clone()
   }
 
@@ -355,6 +357,10 @@ impl Isolate {
   /// Disposes the isolate.  The isolate must not be entered by any
   /// thread to be disposable.
   unsafe fn dispose(&mut self) {
+    IsolateHandle::dispose_isolate(self);
+
+    // No test case in rusty_v8 show this, but there have been situations in
+    // deno where dropping Annex before the states causes a segfault.
     let ptr = self.get_data_int(InternalSlots::States as _) as *mut States;
     if !ptr.is_null() {
       let states = Box::from_raw(ptr);
@@ -362,7 +368,6 @@ impl Isolate {
       drop(states);
     }
 
-    IsolateHandle::dispose_isolate(self);
     v8__Isolate__Dispose(self)
   }
 }
@@ -520,6 +525,9 @@ pub(crate) unsafe fn new_owned_isolate(
 /// Same as Isolate but gets disposed when it goes out of scope.
 pub struct OwnedIsolate(NonNull<Isolate>);
 
+// TODO(ry) unsafe impl Send for OwnedIsolate {}
+// TODO(ry) impl !Sync for OwnedIsolate {}
+
 impl InIsolate for OwnedIsolate {
   fn isolate(&mut self) -> &mut Isolate {
     self.deref_mut()

tests/test_api.rs

@@ -532,53 +532,41 @@ fn thread_safe_handle_drop_after_isolate() {
   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 (tx, rx) = std::sync::mpsc::channel::<bool>();
   let handle = isolate.thread_safe_handle();
-  // Originally run fine.
+  let t = std::thread::spawn(move || {
-  {
+    // allow deno to boot and run
-    let mut hs = v8::HandleScope::new(&mut isolate);
+    std::thread::sleep(std::time::Duration::from_millis(300));
-    let scope = hs.enter();
+    handle.terminate_execution();
-    let context = v8::Context::new(scope);
+    // allow shutdown
-    let mut cs = v8::ContextScope::new(scope, context);
+    std::thread::sleep(std::time::Duration::from_millis(200));
-    let scope = cs.enter();
+    // unless reported otherwise the test should fail after this point
-    let result = eval(scope, context, "true").unwrap();
+    tx.send(false).ok();
-    let true_val = v8::Boolean::new(scope, true).into();
+  });
-    assert!(result.same_value(true_val));
+
-  }
+  let mut hs = v8::HandleScope::new(&mut isolate);
-  // Terminate.
+  let scope = hs.enter();
-  handle.terminate_execution();
+  let context = v8::Context::new(scope);
-  // Below run should fail with terminated knowledge.
+  let mut cs = v8::ContextScope::new(scope, context);
-  {
+  let scope = cs.enter();
-    let mut hs = v8::HandleScope::new(&mut isolate);
+  // Rn an infinite loop, which should be terminated.
-    let scope = hs.enter();
+  let source = v8_str(scope, "for(;;) {}");
-    let context = v8::Context::new(scope);
+  let r = v8::Script::compile(scope, context, source, None);
-    let mut cs = v8::ContextScope::new(scope, context);
+  let mut script = r.unwrap();
-    let scope = cs.enter();
+  let result = script.run(scope, context);
-    let mut try_catch = v8::TryCatch::new(scope);
+  assert!(result.is_none());
-    let tc = try_catch.enter();
+  // TODO assert_eq!(e.to_string(), "Uncaught Error: execution terminated")
-    let _ = eval(scope, context, "true");
+  let msg = rx.recv().expect("execution should be terminated");
-    assert!(tc.has_caught());
+  assert!(!msg);
-    assert!(tc.has_terminated());
+  // Make sure the isolate unusable again.
-  }
+  eval(scope, context, "1+1").expect("execution should be possible again");
-  // Cancel termination.
+  t.join().expect("join t");
-  handle.cancel_terminate_execution();
-  // Works again.
-  {
-    let mut hs = v8::HandleScope::new(&mut isolate);
-    let scope = hs.enter();
-    let context = v8::Context::new(scope);
-    let mut cs = v8::ContextScope::new(scope, context);
-    let scope = cs.enter();
-    let result = eval(scope, context, "true").unwrap();
-    let true_val = v8::Boolean::new(scope, true).into();
-    assert!(result.same_value(true_val));
-  }
 }
 
 // TODO(ry) This test should use threads