Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make --inspect-brk pause on the first line of _user_ code #5250

Merged
merged 1 commit into from
May 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}
};
}
}
piscisaureus marked this conversation as resolved.
Show resolved Hide resolved
}

#[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