Commit Graph
3 Commits
Author SHA1 Message Date
Ben Younes 10810e250c fix(cdp): keep HTTP control plane reachable while V8 JS evaluation blocks (#173)
fix(cdp): unblock HTTP control plane during synchronous V8 execution

Fixes #62.

Previously, the TCP accept loop and CDP processor shared a single-threaded
tokio LocalSet. Long synchronous JS evaluations starved the accept loop,
causing incoming connections to hang in the kernel backlog and effectively
killing the HTTP control plane.

This moves the TCP listener and `/json/*` endpoints to a dedicated OS
thread using `std::net::TcpListener`. HTTP requests are now handled
directly via blocking I/O on this thread, completely bypassing V8.

WebSocket connections are handed off to the LocalSet via a bounded mpsc
channel (capacity 128). If V8 stalls and the queue fills, new WS
connections are dropped (ECONNRESET) rather than buffering unboundedly
to prevent OOMs.

Includes a new integration test to verify the control plane stays
responsive during a 5s JS loop, plus inline documentation clarifying
thread lifetime semantics and a minor test TOCTOU race.
2026-05-24 17:34:55 +00:00
Alexandr KondakovandSGavrl 6dd1e4e65f Fix CDP click submit navigation parity (#69)
* Fix Runtime.evaluate awaitPromise settling

* Fix DOM event navigation side effects

* Fix CDP click-triggered navigation parity

* Strengthen CDP inline script click parity test

* Handle positions-like submit URLs in CDP parity


Co-authored-by: SGavrl <[email protected]>
2026-05-15 20:38:54 +00:00
Timothy Olaleke 85739d3f6a fix(#19): serialize V8 work across pages with process-wide async lock (#36)
* fix(#19): serialize V8 work across pages with process-wide async lock

Concurrent CDP clients (≥5) trip V8's
`heap->isolate() == Isolate::TryGetCurrent()` invariant and abort the
process via V8_Fatal — no Rust panic, just abort(3). Each Page owns its
own JsRuntime/Isolate and they all run on a single OS thread via
LocalSet+spawn_local; when two pages' V8-touching futures interleave
across an .await, V8 trips the check.

This adds a process-wide tokio::sync::Mutex (obscura_js::v8_lock) that
is acquired:

  - At the top of obscura_cdp::dispatch::dispatch (covers every CDP
    handler that may call into a JsRuntime).
  - In server.rs's process_with_interception spawn_local that runs
    page.navigate_with_wait + execute_preload_script (this path
    bypasses dispatch).

The guard keeps each V8-touching block contiguous on the thread: V8
fully exits one Isolate before another page is allowed in. Trade-off:
non-V8 handlers (Browser.*, Storage.*, Emulation.*) also serialize,
but those are cheap and the safety win dominates.

The properly concurrent fix is to pin each JsRuntime to its own OS
thread and message-pass; tracked as the larger #19 follow-up.

Adds an opt-in smoke test (`--ignored`) that boots the CDP server and
runs 5 parallel clients through createTarget+navigate. Note: the
deterministic abort needs subresource-fetch yields (page.rs:285
join_all) which data: URLs skip — full repro requires JS-heavy real
traffic (issue author used scrapegraphai.com).

* fix(#19): suspend other Isolates and defer foreign CDP during interception nav

Address maintainer feedback on PR #36 (comment 4328238087): the V8 fatal
still reproduces with `Fetch.enable patterns:["*"]` + 5 concurrent
`Page.navigate`s. The `obscura_js::v8_lock` mutex was insufficient
because V8's actual invariant is "only one Isolate may be entered per
OS thread", and the mutex can't enforce that across `.await` points
inside V8 ops.

Two distinct paths violated the invariant:

1. Foreign CDP messages dispatched while a nav task was running.
   `process_with_interception`'s `select!` routed every non-Fetch
   message through `process_cdp_message → dispatch → page::handle::*`.
   `get_session_page_mut` then `suspend_js`'es other pages (drops
   their `JsRuntime` → `Isolate::Exit`) while the spawned nav task is
   parked at `op_fetch_url`'s `resolve_rx.await` with its Isolate
   still entered. Drop trips
   `heap->isolate() == Isolate::TryGetCurrent()`.

2. Subsequent navs through `process_with_interception` did not
   suspend prior pages. After nav-1 completes (page-1's JsRuntime
   stays alive in `ctx.pages`), nav-2's `init_js` constructs
   Isolate-2 while Isolate-1 is still current. The next V8 scope
   unwind aborts via `Context::Exit`.

Fix (3 changes in `crates/obscura-cdp/src/server.rs`):

- `cdp_processor`: add a `VecDeque<ServerMessage> deferred` drained
  before each new `rx.recv()`, so deferred messages are processed
  with no nav task in flight.
- `process_with_interception` `select!`: only handle `NewConnection`
  and `Fetch.{continue,fulfill,fail}Request` inline (safe — no V8
  enter on this side); push everything else into `deferred`.
- `process_with_interception` entry: explicitly `suspend_js()` every
  other page in `ctx.pages` before spawning the nav task, mirroring
  the invariant the regular dispatch path already enforces via
  `get_session_page_mut`.

Verification:

- New test `crates/obscura-cdp/tests/concurrent_navigations_with_fetch.rs`
  is the maintainer's exact repro (5 clients, `Fetch.enable patterns:["*"]`,
  concurrent `Page.navigate` to https://example.com, auto-reply
  `Fetch.continueRequest`). Aborts on PR #36 head, passes after fix
  (5/5 stable runs, ~1.2s each).
- Existing data-URL smoke test (`concurrent_navigations`) still passes.
- Full workspace test suite (90 tests) still passes.

Run with:

  cargo test -p obscura-cdp --test concurrent_navigations_with_fetch \
    -- --nocapture --ignored

Honest caveat. This is still the lock/serialize approach — it converts
the abort into latency. The properly concurrent fix (Option 2 in
issue #19: pin each `JsRuntime` to its own OS thread and message-pass)
remains the right long-term direction.

* fix(cdp): bound deferred message queue to prevent OOM during stalled navigation

PR #36 comment 4341743194: process_with_interception deferred foreign
CDP messages into an unbounded VecDeque while a navigation task was in
flight. A stalled navigation (e.g. waiting for Fetch interception
resolution) could accumulate arbitrary queued messages and OOM the
process.

Add MAX_DEFERRED_MESSAGES = 256. When the cap is reached, return an
explicit CDP error (-32000) to the client instead of silently dropping
or growing without bound.

All 90 workspace tests pass.
2026-05-09 18:26:38 +02:00