Skip to content

Commit

Permalink
Make --inspect-brk pause on the first line of _user_ code (#5250)
Browse files Browse the repository at this point in the history
  • Loading branch information
piscisaureus committed May 12, 2020
1 parent e90c95b commit e34a3b6
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 85 deletions.
39 changes: 21 additions & 18 deletions cli/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,6 @@ impl DenoInspector {
pub fn new(
isolate: &mut deno_core::CoreIsolate,
host: SocketAddr,
wait_for_debugger: bool,
) -> Box<Self> {
let deno_core::CoreIsolate {
v8_isolate,
Expand Down Expand Up @@ -406,7 +405,7 @@ impl DenoInspector {
v8::inspector::V8Inspector::create(scope, unsafe { &mut *self_ptr });

let sessions = InspectorSessions::new(self_ptr, new_websocket_rx);
let flags = InspectorFlags::new(wait_for_debugger);
let flags = InspectorFlags::new();
let waker = InspectorWaker::new(scope.isolate().thread_safe_handle());

Self {
Expand All @@ -426,9 +425,6 @@ impl DenoInspector {
self_.context_created(context, Self::CONTEXT_GROUP_ID, context_name);

// Register this inspector with the server thread.
// Note: poll_sessions() might block if we need to wait for a
// debugger front-end to connect. Therefore the server thread must to be
// nofified *before* polling.
InspectorServer::register_inspector(info);

// Poll the session handler so we will get notified whenever there is
Expand Down Expand Up @@ -469,14 +465,9 @@ impl DenoInspector {
replace(&mut self.flags.borrow_mut().session_handshake_done, false);
match poll_result {
Poll::Pending if handshake_done => {
let mut session = sessions.handshake.take().unwrap();
if replace(
&mut self.flags.borrow_mut().waiting_for_session,
false,
) {
session.break_on_first_statement();
}
let session = sessions.handshake.take().unwrap();
sessions.established.push(session);
take(&mut self.flags.borrow_mut().waiting_for_session);
}
Poll::Ready(_) => sessions.handshake = None,
Poll::Pending => break,
Expand Down Expand Up @@ -547,6 +538,21 @@ impl DenoInspector {
};
}
}

/// This function blocks the thread until at least one inspector client has
/// established a websocket connection and successfully completed the
/// handshake. After that, it instructs V8 to pause at the next statement.
pub fn wait_for_session_and_break_on_next_statement(&mut self) {
loop {
match self.sessions.get_mut().established.iter_mut().next() {
Some(session) => break session.break_on_next_statement(),
None => {
self.flags.get_mut().waiting_for_session = true;
let _ = self.poll_sessions(None).unwrap();
}
};
}
}
}

#[derive(Default)]
Expand All @@ -557,11 +563,8 @@ struct InspectorFlags {
}

impl InspectorFlags {
fn new(waiting_for_session: bool) -> RefCell<Self> {
let self_ = Self {
waiting_for_session,
..Default::default()
};
fn new() -> RefCell<Self> {
let self_ = Self::default();
RefCell::new(self_)
}
}
Expand Down Expand Up @@ -753,7 +756,7 @@ impl DenoInspectorSession {
let _ = self.websocket_tx.unbounded_send(msg);
}

pub fn break_on_first_statement(&mut self) {
pub fn break_on_next_statement(&mut self) {
let reason = v8::inspector::StringView::from(&b"debugCommand"[..]);
let detail = v8::inspector::StringView::empty();
self.schedule_pause_on_next_statement(reason, detail);
Expand Down
3 changes: 1 addition & 2 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ use crate::msg::MediaType;
use crate::op_error::OpError;
use crate::ops::io::get_stdio;
use crate::permissions::Permissions;
use crate::state::DebugType;
use crate::state::State;
use crate::tsc::TargetLib;
use crate::worker::MainWorker;
Expand Down Expand Up @@ -140,7 +139,7 @@ fn create_main_worker(
global_state: GlobalState,
main_module: ModuleSpecifier,
) -> Result<MainWorker, ErrBox> {
let state = State::new(global_state, None, main_module, DebugType::Main)?;
let state = State::new(global_state, None, main_module, false)?;

let mut worker = MainWorker::new(
"main".to_string(),
Expand Down
53 changes: 39 additions & 14 deletions cli/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::file_fetcher::SourceFileFetcher;
use crate::global_state::GlobalState;
use crate::global_timer::GlobalTimer;
use crate::import_map::ImportMap;
use crate::inspector::DenoInspector;
use crate::metrics::Metrics;
use crate::op_error::OpError;
use crate::ops::JsonOp;
Expand All @@ -11,6 +12,7 @@ use crate::permissions::Permissions;
use crate::tsc::TargetLib;
use crate::web_worker::WebWorkerHandle;
use deno_core::Buf;
use deno_core::CoreIsolate;
use deno_core::ErrBox;
use deno_core::ModuleLoadId;
use deno_core::ModuleLoader;
Expand All @@ -31,15 +33,6 @@ use std::rc::Rc;
use std::str;
use std::thread::JoinHandle;
use std::time::Instant;
#[derive(Copy, Clone, Eq, PartialEq)]
pub enum DebugType {
/// Can be debugged, will wait for debugger when --inspect-brk given.
Main,
/// Can be debugged, never waits for debugger.
Dependent,
/// No inspector instance is created.
Internal,
}

#[derive(Clone)]
pub struct State(Rc<RefCell<StateInner>>);
Expand All @@ -66,8 +59,9 @@ pub struct StateInner {
pub start_time: Instant,
pub seeded_rng: Option<StdRng>,
pub target_lib: TargetLib,
pub debug_type: DebugType,
pub is_main: bool,
pub is_internal: bool,
pub inspector: Option<Box<DenoInspector>>,
}

impl State {
Expand Down Expand Up @@ -370,7 +364,7 @@ impl State {
global_state: GlobalState,
shared_permissions: Option<Permissions>,
main_module: ModuleSpecifier,
debug_type: DebugType,
is_internal: bool,
) -> Result<Self, ErrBox> {
let import_map: Option<ImportMap> =
match global_state.flags.import_map_path.as_ref() {
Expand Down Expand Up @@ -406,8 +400,9 @@ impl State {
start_time: Instant::now(),
seeded_rng,
target_lib: TargetLib::Main,
debug_type,
is_main: true,
is_internal,
inspector: None,
}));

Ok(Self(state))
Expand Down Expand Up @@ -442,8 +437,9 @@ impl State {
start_time: Instant::now(),
seeded_rng,
target_lib: TargetLib::Worker,
debug_type: DebugType::Dependent,
is_main: false,
is_internal: false,
inspector: None,
}));

Ok(Self(state))
Expand Down Expand Up @@ -512,6 +508,35 @@ impl State {
}
}

pub fn maybe_init_inspector(&self, isolate: &mut CoreIsolate) {
let mut state = self.borrow_mut();

if state.is_internal {
return;
};

let inspector_host = {
let global_state = &state.global_state;
match global_state
.flags
.inspect
.or(global_state.flags.inspect_brk)
{
Some(host) => host,
None => return,
}
};

let inspector = DenoInspector::new(isolate, inspector_host);
state.inspector.replace(inspector);
}

#[inline]
pub fn should_inspector_break_on_first_statement(&self) -> bool {
let state = self.borrow();
state.inspector.is_some() && state.is_main
}

#[cfg(test)]
pub fn mock(main_module: &str) -> State {
let module_specifier = ModuleSpecifier::resolve_url_or_path(main_module)
Expand All @@ -520,7 +545,7 @@ impl State {
GlobalState::mock(vec!["deno".to_string()]),
None,
module_specifier,
DebugType::Main,
false,
)
.unwrap()
}
Expand Down
11 changes: 3 additions & 8 deletions cli/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use crate::permissions::Permissions;
use crate::source_maps::SourceMapGetter;
use crate::startup_data;
use crate::state::State;
use crate::state::*;
use crate::tokio_util;
use crate::version;
use crate::web_worker::WebWorker;
Expand Down Expand Up @@ -351,13 +350,9 @@ impl TsCompiler {
) -> CompilerWorker {
let entry_point =
ModuleSpecifier::resolve_url_or_path("./__$deno$ts_compiler.ts").unwrap();
let worker_state = State::new(
global_state.clone(),
Some(permissions),
entry_point,
DebugType::Internal,
)
.expect("Unable to create worker state");
let worker_state =
State::new(global_state.clone(), Some(permissions), entry_point, true)
.expect("Unable to create worker state");

// Count how many times we start the compiler worker.
global_state.compiler_starts.fetch_add(1, Ordering::SeqCst);
Expand Down
Loading

0 comments on commit e34a3b6

Please sign in to comment.