From fb2841b7ab8efbe27c56703af6c9220d34f10ef9 Mon Sep 17 00:00:00 2001 From: SGavrl <277843151+SGavrl@users.noreply.github.com> Date: Sun, 7 Jun 2026 09:44:49 +0000 Subject: [PATCH] fix: CJK browser_search panic and multi-tab tab_close crash (#257, #258) * Fix UTF-8 char boundary panic in browser_search The search snippet window sliced body[..start] / body[end..] at byte offsets derived from char counts and needle.len(), which can land inside a multi-byte (CJK) character and panic, killing the MCP stdio transport. Snap start/end to char boundaries before slicing and advance past whitespace by the actual char width. Closes #257. * Add setLineDash and friends to the canvas 2D mock Charting libraries (Highcharts, ECharts) call setLineDash/getLineDash/ ellipse every animation frame. The missing setLineDash threw 'is not a function' from a timer each tick, spamming the console. Adds the line-dash methods plus ellipse/roundRect/createConicGradient/ getContextAttributes stubs. Addresses the error spam in #258; the multi-tab isolate-teardown crash is a separate deeper issue. * Fix multi-tab tab_close V8 isolate abort rusty_v8 enters each isolate on creation and requires them dropped in reverse order of creation. The MCP gave each tab its own isolate and dropped them in arbitrary (user-chosen) order on tab_close, so closing a non-newest tab aborted the process (Check failed: heap->isolate() == Isolate::TryGetCurrent()). Pages with active timers (Highcharts/ECharts) made it easy to hit. Enforce the CDP server's single-live-isolate discipline in the MCP: at most one tab's isolate is ever live. page_mut calls a new activate() that suspends every other tab (drops its isolate, keeps its DOM) and resumes the active one. close_tab drops the closed tab's isolate via suspend_js while it is still the only entered isolate, and tool_close suspends all tabs before clearing. Confined to obscura-mcp; the engine, CDP server and obstacle course are untouched, and peak memory drops to one isolate instead of N. Closes #258. --- crates/obscura-js/js/bootstrap.js | 9 +++++ crates/obscura-mcp/src/lib.rs | 59 ++++++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/crates/obscura-js/js/bootstrap.js b/crates/obscura-js/js/bootstrap.js index 5f19af8..b715dd1 100644 --- a/crates/obscura-js/js/bootstrap.js +++ b/crates/obscura-js/js/bootstrap.js @@ -5029,6 +5029,15 @@ class _Canvas2D { createPattern() { return {}; } isPointInPath() { return false; } isPointInStroke() { return false; } + // Line-dash plus a few path/style methods that charting libraries (Highcharts, + // ECharts) call on every animation frame. A missing setLineDash threw + // "is not a function" from a timer each tick, spamming errors (#258). + setLineDash() {} + getLineDash() { return []; } + ellipse() {} + roundRect() {} + createConicGradient() { return { addColorStop(){} }; } + getContextAttributes() { return { alpha: true, desynchronized: false, colorSpace: "srgb", willReadFrequently: false }; } } Element.prototype.getContext = function getContext(type) { diff --git a/crates/obscura-mcp/src/lib.rs b/crates/obscura-mcp/src/lib.rs index aebb17b..9543068 100644 --- a/crates/obscura-mcp/src/lib.rs +++ b/crates/obscura-mcp/src/lib.rs @@ -102,6 +102,7 @@ impl BrowserState { self.active_tab = Some(id); } let id = self.active_tab.as_ref().unwrap().clone(); + self.activate(&id); self.tabs.get_mut(&id).expect("active tab must exist") } @@ -114,6 +115,34 @@ impl BrowserState { id } + /// Enforce the single-live-isolate invariant. rusty_v8 enters each V8 + /// isolate on creation and requires isolates be dropped in reverse order of + /// creation, so keeping more than one tab's isolate live at once and then + /// dropping a non-newest one aborts the whole process (#258). Suspend every + /// other tab (drops its isolate, keeps its DOM in self.dom) and make the + /// active tab the only live isolate, mirroring the CDP server's + /// Dispatcher::get_session_page_mut. + fn activate(&mut self, tab_id: &str) { + for (id, page) in self.tabs.iter_mut() { + if id.as_str() != tab_id && page.has_js() { + page.suspend_js(); + } + } + if let Some(page) = self.tabs.get_mut(tab_id) { + page.resume_js(); + } + } + + /// Close a tab without breaking the LIFO isolate-drop rule. suspend_js drops + /// this tab's isolate (if it is the live one) while it is still the only + /// entered isolate, so the following remove disposes no isolate (#258). + fn close_tab(&mut self, tab_id: &str) -> bool { + if let Some(page) = self.tabs.get_mut(tab_id) { + page.suspend_js(); + } + self.tabs.remove(tab_id).is_some() + } + /// Resolve `ref=eN` to a CSS selector that uniquely targets the /// element. Snapshot writes `data-obscura-ref="eN"` onto every /// interactable, so the attribute survives across calls as long as @@ -903,6 +932,12 @@ fn tool_console_messages(state: &BrowserState) -> Result { } fn tool_close(state: &mut BrowserState) -> Result { + // Drop the one live isolate (if any) via suspend_js before clearing, so the + // map drop disposes no isolate and the LIFO rule holds regardless of the + // BTreeMap's ascending drop order (#258). + for page in state.tabs.values_mut() { + page.suspend_js(); + } state.tabs.clear(); state.active_tab = None; state.console_messages.clear(); @@ -1513,7 +1548,7 @@ fn tool_tab_close(args: &Value, state: &mut BrowserState) -> Result Result let mut idx = 0; while let Some(pos) = haystack[idx..].find(&needle) { let abs = idx + pos; - let start = abs.saturating_sub(context); - let end = (abs + needle.len() + context).min(body.len()); - // body and haystack share byte offsets even when lowercased (only ASCII fold). - // For safety on non-ASCII, fall back to char-boundary trimming. - let start = body[..start].rfind(|c: char| c.is_whitespace()) - .map(|i| i + 1).unwrap_or(start); - let end = body[end..].find(|c: char| c.is_whitespace()) - .map(|i| end + i).unwrap_or(end); + let mut start = abs.saturating_sub(context); + let mut end = (abs + needle.len() + context).min(body.len()); + // start/end are byte offsets derived from char counts and needle.len(), + // so they can land inside a multi-byte (CJK) character. Snap to char + // boundaries before slicing or body[..start] panics (#257). + while start > 0 && !body.is_char_boundary(start) { start -= 1; } + while end < body.len() && !body.is_char_boundary(end) { end += 1; } + // Trim inward to the nearest whitespace so snippets start/end on words. + if let Some(i) = body[..start].rfind(|c: char| c.is_whitespace()) { + start = i + body[i..].chars().next().map_or(1, char::len_utf8); + } + if let Some(i) = body[end..].find(|c: char| c.is_whitespace()) { + end += i; + } let snippet = body.get(start..end).unwrap_or("").trim().replace('\n', " "); out.push(json!({ "offset": abs,