Skip to content

Commit

Permalink
core: Allow terminating an Isolate from another thread (denoland#1982)
Browse files Browse the repository at this point in the history
  • Loading branch information
fd authored and ry committed Mar 21, 2019
1 parent 94405bb commit 93793dc
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 1 deletion.
106 changes: 105 additions & 1 deletion core/isolate.rs
Expand Up @@ -11,7 +11,7 @@ use futures::Poll;
use libc::c_void;
use std::ffi::CStr;
use std::ffi::CString;
use std::sync::{Once, ONCE_INIT};
use std::sync::{Arc, Mutex, Once, ONCE_INIT};

pub type Buf = Box<[u8]>;
pub type Op = dyn Future<Item = Buf, Error = ()> + Send;
Expand Down Expand Up @@ -85,6 +85,7 @@ pub trait Behavior {
/// JavaScript.
pub struct Isolate<B: Behavior> {
libdeno_isolate: *const libdeno::isolate,
shared_libdeno_isolate: Arc<Mutex<Option<*const libdeno::isolate>>>,
behavior: B,
needs_init: bool,
shared: SharedQueue,
Expand All @@ -96,6 +97,9 @@ unsafe impl<B: Behavior> Send for Isolate<B> {}

impl<B: Behavior> Drop for Isolate<B> {
fn drop(&mut self) {
// remove shared_libdeno_isolate reference
*self.shared_libdeno_isolate.lock().unwrap() = None;

unsafe { libdeno::deno_delete(self.libdeno_isolate) }
}
}
Expand Down Expand Up @@ -130,6 +134,7 @@ impl<B: Behavior> Isolate<B> {

let mut core_isolate = Self {
libdeno_isolate,
shared_libdeno_isolate: Arc::new(Mutex::new(Some(libdeno_isolate))),
behavior,
shared,
needs_init,
Expand All @@ -148,6 +153,13 @@ impl<B: Behavior> Isolate<B> {
core_isolate
}

/// Get a thread safe handle on the isolate.
pub fn shared_isolate_handle(&mut self) -> IsolateHandle {
IsolateHandle {
shared_libdeno_isolate: self.shared_libdeno_isolate.clone(),
}
}

/// Executes a bit of built-in JavaScript to provide Deno._sharedQueue.
pub fn shared_init(&mut self) {
if self.needs_init {
Expand Down Expand Up @@ -452,6 +464,28 @@ impl<B: Behavior> Future for Isolate<B> {
}
}

/// IsolateHandle is a thread safe handle on an Isolate. It exposed thread safe V8 functions.
#[derive(Clone)]
pub struct IsolateHandle {
shared_libdeno_isolate: Arc<Mutex<Option<*const libdeno::isolate>>>,
}

unsafe impl Send for IsolateHandle {}

impl IsolateHandle {
/// Terminate the execution of any currently running javascript.
/// After terminating execution it is probably not wise to continue using
/// the isolate.
pub fn terminate_execution(&self) {
unsafe {
match *self.shared_libdeno_isolate.lock().unwrap() {
Some(isolate) => libdeno::deno_terminate_execution(isolate),
None => (),
}
}
}
}

pub fn js_check(r: Result<(), JSError>) {
if let Err(e) = r {
panic!(e.to_string());
Expand Down Expand Up @@ -698,6 +732,76 @@ mod tests {
js_check(isolate.execute("send1.js", "assert(nrecv === 2);"));
}

#[test]
fn terminate_execution() {
let (tx, rx) = std::sync::mpsc::channel::<bool>();
let tx_clone = tx.clone();

let mut isolate = TestBehavior::setup(TestBehaviorMode::AsyncImmediate);
let shared = isolate.shared_isolate_handle();

let t1 = std::thread::spawn(move || {
// allow deno to boot and run
std::thread::sleep(std::time::Duration::from_millis(100));

// terminate execution
shared.terminate_execution();

// allow shutdown
std::thread::sleep(std::time::Duration::from_millis(100));

// unless reported otherwise the test should fail after this point
tx_clone.send(false).ok();
});

let t2 = std::thread::spawn(move || {
// run an infinite loop
let res = isolate.execute(
"infinite_loop.js",
r#"
let i = 0;
while (true) { i++; }
"#,
);

// execute() terminated, which means terminate_execution() was successful.
tx.send(true).ok();

if let Err(e) = res {
assert_eq!(e.to_string(), "Uncaught Error: execution terminated");
} else {
panic!("should return an error");
}

// make sure the isolate is still unusable
let res = isolate.execute("simple.js", "1+1;");
if let Err(e) = res {
assert_eq!(e.to_string(), "Uncaught Error: execution terminated");
} else {
panic!("should return an error");
}
});

if !rx.recv().unwrap() {
panic!("should have terminated")
}

t1.join().unwrap();
t2.join().unwrap();
}

#[test]
fn dangling_shared_isolate() {
let shared = {
// isolate is dropped at the end of this block
let mut isolate = TestBehavior::setup(TestBehaviorMode::AsyncImmediate);
isolate.shared_isolate_handle()
};

// this should not SEGFAULT
shared.terminate_execution();
}

#[test]
fn overflow_req_sync() {
let mut isolate = TestBehavior::setup(TestBehaviorMode::OverflowReqSync);
Expand Down
1 change: 1 addition & 0 deletions core/libdeno.rs
Expand Up @@ -155,6 +155,7 @@ extern "C" {
js_filename: *const c_char,
js_source: *const c_char,
);
pub fn deno_terminate_execution(i: *const isolate);

// Modules

Expand Down
26 changes: 26 additions & 0 deletions libdeno/exceptions.cc
Expand Up @@ -174,6 +174,25 @@ std::string EncodeExceptionAsJSON(v8::Local<v8::Context> context,
void HandleException(v8::Local<v8::Context> context,
v8::Local<v8::Value> exception) {
v8::Isolate* isolate = context->GetIsolate();

// TerminateExecution was called
if (isolate->IsExecutionTerminating()) {
// cancel exception termination so that the exception can be created
isolate->CancelTerminateExecution();

// maybe make a new exception object
if (exception->IsNullOrUndefined()) {
exception = v8::Exception::Error(v8_str("execution terminated"));
}

// handle the exception as if it is a regular exception
HandleException(context, exception);

// re-enable exception termination
isolate->TerminateExecution();
return;
}

DenoIsolate* d = DenoIsolate::FromIsolate(isolate);
std::string json_str = EncodeExceptionAsJSON(context, exception);
CHECK_NOT_NULL(d);
Expand All @@ -183,6 +202,13 @@ void HandleException(v8::Local<v8::Context> context,
void HandleExceptionMessage(v8::Local<v8::Context> context,
v8::Local<v8::Message> message) {
v8::Isolate* isolate = context->GetIsolate();

// TerminateExecution was called
if (isolate->IsExecutionTerminating()) {
HandleException(context, v8::Undefined(isolate));
return;
}

DenoIsolate* d = DenoIsolate::FromIsolate(isolate);
std::string json_str = EncodeMessageAsJSON(context, message);
CHECK_NOT_NULL(d);
Expand Down

0 comments on commit 93793dc

Please sign in to comment.