diff --git a/crates/obscura-cdp/src/dispatch.rs b/crates/obscura-cdp/src/dispatch.rs index 30fa947..de2ff67 100644 --- a/crates/obscura-cdp/src/dispatch.rs +++ b/crates/obscura-cdp/src/dispatch.rs @@ -51,6 +51,14 @@ pub struct CdpContext { // caps how many bodies (and how many bytes) can be held at once, evicting // the oldest, so an abandoned or disconnected stream cannot leak unbounded. pub io_streams: crate::domains::io::IoStreamStore, + /// Serializes V8 work within THIS connection. With the thread-per-connection + /// server (#430) each connection runs on its own OS thread, so isolates never + /// collide across connections; this per-connection lock keeps a connection's + /// own nav task and command dispatch from interleaving two of its pages' + /// isolates on that one thread. It is deliberately per-connection, not a + /// process-wide lock, so connections run in parallel (measured ~2x at + /// concurrency 2, ~3x at 4) instead of serializing all V8 on one mutex. + pub v8_lock: Arc>, } impl CdpContext { @@ -131,6 +139,7 @@ impl CdpContext { valid_context_ids, next_isolated_context_id: 100, io_streams: crate::domains::io::IoStreamStore::default(), + v8_lock: Arc::new(tokio::sync::Mutex::new(())), } } @@ -175,6 +184,7 @@ impl CdpContext { valid_context_ids, next_isolated_context_id: 100, io_streams: crate::domains::io::IoStreamStore::default(), + v8_lock: Arc::new(tokio::sync::Mutex::new(())), } } @@ -323,7 +333,10 @@ pub async fn dispatch(req: &CdpRequest, ctx: &mut CdpContext) -> CdpResponse { let _v8_guard = if is_v8_free_method(&req.method) { None } else { - Some(obscura_js::v8_lock::global().lock().await) + // Per-connection lock (owned guard, so it does not borrow `ctx`): keeps + // this connection's own V8 work contiguous on its thread without + // serializing other connections (#430). + Some(ctx.v8_lock.clone().lock_owned().await) }; // Per-command V8 watchdog. The lock above keeps each handler contiguous on diff --git a/crates/obscura-cdp/src/server.rs b/crates/obscura-cdp/src/server.rs index a9c2e39..3764ac8 100644 --- a/crates/obscura-cdp/src/server.rs +++ b/crates/obscura-cdp/src/server.rs @@ -673,13 +673,15 @@ async fn process_with_interception( let (nav_done_tx, mut nav_done_rx) = mpsc::channel::<(obscura_browser::Page, Result<(), String>)>(1); let url_owned = url.to_string(); + let nav_v8_lock = ctx.v8_lock.clone(); tokio::task::spawn_local(async move { - // Issue #19: serialize V8 work across pages. The interception path - // spawns navigation here while the parent task continues to pump - // CDP messages via `dispatch` (which also acquires this lock); both - // sides must coordinate or V8 aborts the process at concurrency >= 5. - let _v8_guard = obscura_js::v8_lock::global().lock().await; + // Issue #19: serialize this connection's V8 work across its pages. This + // nav task runs while the connection's processor keeps pumping other CDP + // messages via `dispatch` (which takes the same per-connection lock), so + // both sides coordinate on one page's isolate at a time on this thread. + // The lock is per-connection, so other connections are unaffected (#430). + let _v8_guard = nav_v8_lock.lock_owned().await; // Preloads (addBinding shims, addScriptToEvaluateOnNewDocument sources) // must run BEFORE the page's own scripts (CDP contract). Hand them // to the page so navigate_single can inject them at the right point. diff --git a/crates/obscura-js/src/lib.rs b/crates/obscura-js/src/lib.rs index 5217f63..c9919f0 100644 --- a/crates/obscura-js/src/lib.rs +++ b/crates/obscura-js/src/lib.rs @@ -6,7 +6,6 @@ pub mod module_loader; pub mod runtime; pub mod ops; pub mod v8_flags; -pub mod v8_lock; pub mod markdown; pub use markdown::HTML_TO_MARKDOWN_JS; diff --git a/crates/obscura-js/src/v8_lock.rs b/crates/obscura-js/src/v8_lock.rs deleted file mode 100644 index b598821..0000000 --- a/crates/obscura-js/src/v8_lock.rs +++ /dev/null @@ -1,27 +0,0 @@ -//! Process-wide async lock that serializes V8 work across all `JsRuntime`s. -//! -//! V8's invariant: on any given OS thread, only one `Isolate` may be entered -//! at a time, and HandleScope/ContextScope stacks must unwind on the Isolate -//! they belong to. Obscura's CDP server runs every `JsRuntime` (one per Page) -//! on a single OS thread via `tokio::task::LocalSet` + `spawn_local`. As soon -//! as two pages' V8-touching futures interleave across an `.await`, V8 trips -//! its `heap->isolate() == Isolate::TryGetCurrent()` check and aborts the -//! whole process (no Rust panic; `V8_Fatal` calls `abort(3)`). -//! -//! Acquiring this lock around any block that calls `JsRuntime::execute_script` -//! or `JsRuntime::run_event_loop` keeps that block contiguous on the thread: -//! V8 fully exits the prior Isolate before the next page is allowed in. This -//! converts the abort into latency. It is the issue-19 "Option 1" fix. -//! -//! The properly concurrent fix is to pin each `JsRuntime` to its own OS -//! thread (issue-19 "Option 2"); that's a larger refactor tracked separately. - -use std::sync::OnceLock; -use tokio::sync::Mutex; - -static V8_LOCK: OnceLock> = OnceLock::new(); - -/// Returns the process-wide V8 serialization lock. -pub fn global() -> &'static Mutex<()> { - V8_LOCK.get_or_init(|| Mutex::new(())) -}