diff options
| -rw-r--r-- | cli/tests/integration/repl_tests.rs | 17 | ||||
| -rw-r--r-- | core/01_core.js | 1 | ||||
| -rw-r--r-- | core/inspector.rs | 10 | ||||
| -rw-r--r-- | core/ops_builtin_v8.rs | 24 | ||||
| -rw-r--r-- | core/runtime.rs | 58 | ||||
| -rw-r--r-- | ext/web/02_event.js | 3 | ||||
| -rw-r--r-- | runtime/inspector_server.rs | 3 | ||||
| -rw-r--r-- | runtime/worker.rs | 4 |
8 files changed, 76 insertions, 44 deletions
diff --git a/cli/tests/integration/repl_tests.rs b/cli/tests/integration/repl_tests.rs index 94cbef3c9..07b7575a0 100644 --- a/cli/tests/integration/repl_tests.rs +++ b/cli/tests/integration/repl_tests.rs @@ -844,3 +844,20 @@ fn pty_tab_handler() { assert_not_contains!(output, "atob"); }); } + +#[test] +fn repl_report_error() { + let (out, err) = util::run_and_collect_output( + true, + "repl", + Some(vec![ + r#"console.log(1); reportError(new Error("foo")); console.log(2);"#, + ]), + Some(vec![("NO_COLOR".to_owned(), "1".to_owned())]), + false, + ); + + // TODO(nayeemrmn): The REPL should report event errors and rejections. + assert_contains!(out, "1\n2\nundefined\n"); + assert!(err.is_empty()); +} diff --git a/core/01_core.js b/core/01_core.js index c7ced9b82..ab9722bc1 100644 --- a/core/01_core.js +++ b/core/01_core.js @@ -314,7 +314,6 @@ error, ) => ops.op_abort_wasm_streaming(rid, error), destructureError: (error) => ops.op_destructure_error(error), - terminate: (exception) => ops.op_terminate(exception), opNames: () => ops.op_op_names(), eventLoopHasMoreWork: () => ops.op_event_loop_has_more_work(), setPromiseRejectCallback: (fn) => ops.op_set_promise_reject_callback(fn), diff --git a/core/inspector.rs b/core/inspector.rs index 6a254b76c..b1047d421 100644 --- a/core/inspector.rs +++ b/core/inspector.rs @@ -150,7 +150,7 @@ impl JsRuntimeInspector { pub fn new( isolate: &mut v8::OwnedIsolate, context: v8::Global<v8::Context>, - ) -> Box<Self> { + ) -> Rc<RefCell<Self>> { let scope = &mut v8::HandleScope::new(isolate); let (new_session_tx, new_session_rx) = @@ -162,7 +162,7 @@ impl JsRuntimeInspector { let waker = InspectorWaker::new(scope.thread_safe_handle()); // Create JsRuntimeInspector instance. - let mut self_ = Box::new(Self { + let self__ = Rc::new(RefCell::new(Self { v8_inspector_client, v8_inspector: Default::default(), sessions: RefCell::new(SessionContainer::temporary_placeholder()), @@ -170,7 +170,8 @@ impl JsRuntimeInspector { flags: Default::default(), waker, deregister_tx: None, - }); + })); + let mut self_ = self__.borrow_mut(); self_.v8_inspector = Rc::new(RefCell::new( v8::inspector::V8Inspector::create(scope, &mut *self_).into(), )); @@ -192,8 +193,9 @@ impl JsRuntimeInspector { // Poll the session handler so we will get notified whenever there is // new incoming debugger activity. let _ = self_.poll_sessions(None).unwrap(); + drop(self_); - self_ + self__ } pub fn has_active_sessions(&self) -> bool { diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs index f8b59a760..abdbdedc8 100644 --- a/core/ops_builtin_v8.rs +++ b/core/ops_builtin_v8.rs @@ -43,7 +43,7 @@ pub(crate) fn init_builtins_v8() -> Vec<OpDecl> { op_set_wasm_streaming_callback::decl(), op_abort_wasm_streaming::decl(), op_destructure_error::decl(), - op_terminate::decl(), + op_dispatch_exception::decl(), op_op_names::decl(), op_apply_source_map::decl(), op_set_format_exception_callback::decl(), @@ -718,13 +718,27 @@ fn op_destructure_error( JsError::from_v8_exception(scope, error.v8_value) } +/// Effectively throw an uncatchable error. This will terminate runtime +/// execution before any more JS code can run, except in the REPL where it +/// should just output the error to the console. #[op(v8)] -fn op_terminate(scope: &mut v8::HandleScope, exception: serde_v8::Value) { +fn op_dispatch_exception( + scope: &mut v8::HandleScope, + exception: serde_v8::Value, +) { let state_rc = JsRuntime::state(scope); let mut state = state_rc.borrow_mut(); - state.explicit_terminate_exception = - Some(v8::Global::new(scope, exception.v8_value)); - scope.terminate_execution(); + state + .dispatched_exceptions + .push_front(v8::Global::new(scope, exception.v8_value)); + // Only terminate execution if there are no inspector sessions. + match state.inspector().try_borrow() { + Ok(inspector) if !inspector.has_active_sessions() => { + scope.terminate_execution(); + } + // If the inspector is borrowed at this time, assume an inspector is active. + _ => {} + } } #[op(v8)] diff --git a/core/runtime.rs b/core/runtime.rs index 5769bbb3b..b1d8f3d80 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -36,6 +36,7 @@ use std::any::Any; use std::cell::RefCell; use std::collections::HashMap; use std::collections::HashSet; +use std::collections::VecDeque; use std::ffi::c_void; use std::mem::forget; use std::option::Option; @@ -80,9 +81,6 @@ pub struct JsRuntime { // This is an Option<OwnedIsolate> instead of just OwnedIsolate to workaround // a safety issue with SnapshotCreator. See JsRuntime::drop. v8_isolate: Option<v8::OwnedIsolate>, - // This is an Option<Box<JsRuntimeInspector> instead of just Box<JsRuntimeInspector> - // to workaround a safety issue. See JsRuntime::drop. - inspector: Option<Box<JsRuntimeInspector>>, snapshot_creator: Option<v8::SnapshotCreator>, has_snapshotted: bool, built_from_snapshot: bool, @@ -182,19 +180,18 @@ pub(crate) struct JsRuntimeState { pub(crate) op_ctxs: Box<[OpCtx]>, pub(crate) shared_array_buffer_store: Option<SharedArrayBufferStore>, pub(crate) compiled_wasm_module_store: Option<CompiledWasmModuleStore>, - /// The error that was passed to an explicit `Deno.core.terminate` call. + /// The error that was passed to an `op_dispatch_exception` call. /// It will be retrieved by `exception_to_err_result` and used as an error /// instead of any other exceptions. - pub(crate) explicit_terminate_exception: Option<v8::Global<v8::Value>>, + // TODO(nayeemrmn): This is polled in `exception_to_err_result()` which is + // flimsy. Try to poll it similarly to `pending_promise_exceptions`. + pub(crate) dispatched_exceptions: VecDeque<v8::Global<v8::Value>>, + inspector: Option<Rc<RefCell<JsRuntimeInspector>>>, waker: AtomicWaker, } impl Drop for JsRuntime { fn drop(&mut self) { - // The Isolate object must outlive the Inspector object, but this is - // currently not enforced by the type system. - self.inspector.take(); - if let Some(creator) = self.snapshot_creator.take() { // TODO(ry): in rusty_v8, `SnapShotCreator::get_owned_isolate()` returns // a `struct OwnedIsolate` which is not actually owned, hence the need @@ -421,7 +418,8 @@ impl JsRuntime { op_state: op_state.clone(), op_ctxs, have_unpolled_ops: false, - explicit_terminate_exception: None, + dispatched_exceptions: Default::default(), + inspector: Some(inspector), waker: AtomicWaker::new(), }))); @@ -434,7 +432,6 @@ impl JsRuntime { let mut js_runtime = Self { v8_isolate: Some(isolate), - inspector: Some(inspector), snapshot_creator: maybe_snapshot_creator, has_snapshotted: false, built_from_snapshot: has_startup_snapshot, @@ -466,8 +463,8 @@ impl JsRuntime { self.v8_isolate.as_mut().unwrap() } - pub fn inspector(&mut self) -> &mut Box<JsRuntimeInspector> { - self.inspector.as_mut().unwrap() + pub fn inspector(&mut self) -> Rc<RefCell<JsRuntimeInspector>> { + Self::state(self.v8_isolate()).borrow().inspector() } pub fn global_realm(&mut self) -> JsRealm { @@ -740,7 +737,7 @@ impl JsRuntime { state.borrow_mut().global_realm.take(); - self.inspector.take(); + state.borrow_mut().inspector.take(); // Overwrite existing ModuleMap to drop v8::Global handles self @@ -941,8 +938,7 @@ impl JsRuntime { wait_for_inspector: bool, ) -> Poll<Result<(), Error>> { // We always poll the inspector first - let _ = self.inspector().poll_unpin(cx); - + let _ = self.inspector().borrow_mut().poll_unpin(cx); let state_rc = Self::state(self.v8_isolate()); let module_map_rc = Self::module_map(self.v8_isolate()); { @@ -1003,11 +999,8 @@ impl JsRuntime { self.evaluate_pending_module(); let pending_state = Self::event_loop_pending_state(self.v8_isolate()); - let inspector_has_active_sessions = self - .inspector - .as_ref() - .map(|i| i.has_active_sessions()) - .unwrap_or(false); + let inspector_has_active_sessions = + self.inspector().borrow_mut().has_active_sessions(); if !pending_state.is_pending() && !maybe_scheduling { if wait_for_inspector && inspector_has_active_sessions { @@ -1144,6 +1137,10 @@ where } impl JsRuntimeState { + pub(crate) fn inspector(&self) -> Rc<RefCell<JsRuntimeInspector>> { + self.inspector.as_ref().unwrap().clone() + } + /// Called by `bindings::host_import_module_dynamically_callback` /// after initiating new dynamic import load. pub fn notify_new_dynamic_import(&mut self) { @@ -1167,17 +1164,17 @@ pub(crate) fn exception_to_err_result<'s, T>( scope.cancel_terminate_execution(); let mut exception = exception; { - // If the termination is the result of a `Deno.core.terminate` call, we want + // If termination is the result of a `op_dispatch_exception` call, we want // to use the exception that was passed to it rather than the exception that // was passed to this function. let mut state = state_rc.borrow_mut(); exception = state - .explicit_terminate_exception - .take() + .dispatched_exceptions + .pop_back() .map(|exception| v8::Local::new(scope, exception)) .unwrap_or_else(|| { // Maybe make a new exception object. - if exception.is_null_or_undefined() { + if was_terminating_execution && exception.is_null_or_undefined() { let message = v8::String::new(scope, "execution terminated").unwrap(); v8::Exception::error(scope, message) } else { @@ -1385,10 +1382,11 @@ impl JsRuntime { // Update status after evaluating. status = module.get_status(); - let explicit_terminate_exception = - state_rc.borrow_mut().explicit_terminate_exception.take(); - if let Some(exception) = explicit_terminate_exception { - let exception = v8::Local::new(tc_scope, exception); + let has_dispatched_exception = + !state_rc.borrow_mut().dispatched_exceptions.is_empty(); + if has_dispatched_exception { + // This will be overrided in `exception_to_err_result()`. + let exception = v8::undefined(tc_scope).into(); let pending_mod_evaluate = { let mut state = state_rc.borrow_mut(); state.pending_mod_evaluate.take().unwrap() @@ -2702,7 +2700,7 @@ pub mod tests { Deno.core.setPromiseRejectCallback(() => { return false; }); - a = 1 + 2; + a = 1 + 2; "#, ) .unwrap(); diff --git a/ext/web/02_event.js b/ext/web/02_event.js index 105c7d3c4..ee64b3719 100644 --- a/ext/web/02_event.js +++ b/ext/web/02_event.js @@ -8,6 +8,7 @@ ((window) => { const core = window.Deno.core; + const ops = core.ops; const webidl = window.__bootstrap.webidl; const { DOMException } = window.__bootstrap.domException; const consoleInternal = window.__bootstrap.console; @@ -1451,7 +1452,7 @@ }); // Avoid recursing `reportException()` via error handlers more than once. if (reportExceptionStackedCalls > 1 || window.dispatchEvent(event)) { - core.terminate(error); + ops.op_dispatch_exception(error); } reportExceptionStackedCalls--; } diff --git a/runtime/inspector_server.rs b/runtime/inspector_server.rs index dde5cea02..f29eec2e2 100644 --- a/runtime/inspector_server.rs +++ b/runtime/inspector_server.rs @@ -69,7 +69,8 @@ impl InspectorServer { js_runtime: &mut JsRuntime, should_break_on_first_statement: bool, ) { - let inspector = js_runtime.inspector(); + let inspector_rc = js_runtime.inspector(); + let mut inspector = inspector_rc.borrow_mut(); let session_sender = inspector.get_session_sender(); let deregister_rx = inspector.add_deregister_handler(); let info = InspectorInfo::new( diff --git a/runtime/worker.rs b/runtime/worker.rs index e0f54e09d..79b1b5537 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -294,6 +294,7 @@ impl MainWorker { self .js_runtime .inspector() + .borrow_mut() .wait_for_session_and_break_on_next_statement() } } @@ -301,8 +302,7 @@ impl MainWorker { /// Create new inspector session. This function panics if Worker /// was not configured to create inspector. pub async fn create_inspector_session(&mut self) -> LocalInspectorSession { - let inspector = self.js_runtime.inspector(); - inspector.create_local_session() + self.js_runtime.inspector().borrow().create_local_session() } pub fn poll_event_loop( |