Fix/v8 watchdog panic safety (#239)
* dom: complete detached documents; add ProgressEvent and parseHTMLUnsafe
createHTMLDocument returned a document whose body was null: parsing the
skeleton string as innerHTML of <html> collapsed through the fragment parser
and kept only <title>. Build head>title and body explicitly instead. The
DOMParser doc node also gains appendChild/removeChild/insertBefore and a
doctype getter, and createDocument carries its doctype argument.
This lets dom/common.js setupRangeTests() finish (it builds fixtures with
new Document(), createHTMLDocument(), createDocument()) instead of aborting at
harness setup, so the dom/ranges files run. Range-collapse 186/186,
Range-attributes 1/1, Range-cloneRange 55/62, StaticRange-constructor 14/17.
The large O(ranges x points) matrix files (Range-set, comparePoint,
compareBoundaryPoints) still time out: the boundary-point comparison walks the
tree through per-step DOM ops, which is too slow for those matrices. Speeding
that up (a Rust node-position op) is a separate change.
Also add globalThis.ProgressEvent (Event subclass with lengthComputable/
loaded/total) and the static Document.parseHTMLUnsafe(html).
* perf(dom): compute Range tree-order in Rust ops
The Range boundary-point algorithms compared nodes by walking ancestor chains
and sibling lists from JS, one DOM op per step. The dom/ranges matrices
(testRanges x testPoints, thousands of comparisons) made that time out.
Add node_index, compare_order, and node_root ops that do the walk in Rust over
the DOM tree (slab lookups, no per-step FFI), and use them in _rngNodeIndex,
_rngOrder, and _rngRoot. Range-comparePoint 3534/5580, Range-isPointInRange
5197/5733, Range-compareBoundaryPoints 6441/9313, all completing in under 10s
where they previously did not finish. Not on any engine hot path (querySelector,
mutation, navigation, fetch do not use these).
* fileapi: real byte-backed Blob/File, FileReader state machine, Request.blob
Blob was a one-line stub that string-joined its parts (wrong size for binary
or multibyte input) and exposed only text(). Replace it with a Uint8Array-backed
Blob: normalizes string/ArrayBuffer/typed-array/Blob parts, correct size,
slice(), text()/arrayBuffer()/bytes(), type normalization (lowercased, cleared
when non-ASCII), the endings option (native CRLF normalization), and
Symbol.toStringTag. File adds name/lastModified, the 2-argument requirement, and
its own toStringTag.
FileReader becomes a real state machine: EMPTY/LOADING/DONE, throws
InvalidStateError when a read starts while already LOADING, and fires
loadstart/progress/load/loadend (and abort) to both on* handlers and
addEventListener. readAsText/DataURL/ArrayBuffer/BinaryString produce correct
results. Request gains blob().
FileReader-multiple-reads 6/6, File-constructor-endings 10/11, Blob-slice
141/150.
* encoding: TextDecoder supports legacy charsets via encoding_rs
TextDecoder was a UTF-8-only JS shim that ignored its label, so
new TextDecoder('shift_jis'|'euc-jp'|'big5'|'gbk'|'windows-1252'|'iso-8859-1')
.decode() returned garbage. That matters for scraping: pages and scripts decode
legacy-encoded bytes (XHR/fetch responses, File/Blob text) all the time.
Back it with encoding_rs (already used by the net layer for response bodies):
obscura-net gains label_name() and decode_with_label(), exposed to JS as
op_encoding_for_label and op_text_decode. The constructor validates the label
(RangeError) and sets the canonical .encoding/.fatal/.ignoreBOM; decode()
supports fatal mode (TypeError on invalid input) and BOM handling.
Plain UTF-8 non-fatal decoding keeps a pure-JS fast path (no op, no JSON), so
Response.text()/Blob.text() on large bodies are unchanged; the op only runs for
legacy encodings or fatal mode.
textdecoder-labels 222/222, textdecoder-fatal 36/36, byte-order-marks 3/3,
utf16-surrogates 10/10, single-byte-decoder 168/504.
* selection: real Selection over Range (single instance per document)
getSelection() returned a fresh plain object each call, so it was not a Selection
instance, had no identity (window.getSelection() !== document.getSelection()),
and was missing removeRange/empty/collapseToStart/collapseToEnd/setPosition/
setBaseAndExtent/modify while collapse/extend/getRangeAt were broken stubs. That
was ~4862 failing 'selection[method] is not a function' subtests.
Replace it with a real Selection class built on the live Range: one cached
instance per document (window.getSelection() === document.getSelection(), null
for documents with no defaultView), an internal range + direction, derived
anchor/focus/isCollapsed/type/rangeCount, and the full method set
(getRangeAt with IndexSizeError, addRange, removeRange, removeAllRanges/empty,
collapse/setPosition, collapseToStart/End, extend, setBaseAndExtent,
selectAllChildren, containsNode, toString) with the spec DOMException names.
modify() stays a no-op (needs visual layout). No ops.rs changes; not on any hot
path.
setBaseAndExtent 120/120, removeAllRanges 104/104, collapseToStartEnd 51/51,
type 26/26, extend-exception 1/1.
* forms: HTMLInputElement valueAsNumber/valueAsDate/stepUp/stepDown
These were absent. Implement the number/date conversion API for the applicable
types (number, range, date, month, week, time, datetime-local): parsers and
formatters per type (Date.UTC for date/datetime-local, ISO-week Monday math for
week, months-since-epoch for month), valueAsNumber get/set, valueAsDate get/set,
and stepUp/stepDown(n) with the step scale/default per type, step-base snapping,
and min/max clamping. Non-applicable types and step="any" throw InvalidStateError;
non-finite valueAsNumber throws TypeError; range with empty value reads as the
midpoint (50 for 0..100). Plus min/max/step attribute reflection.
All on Element accessors gated by input type, only evaluated on explicit access;
not on any hot path.
input-stepup 51/53, input-valueasnumber 60/64, input-valueasdate 27/30.
* perf(encoding): skip label op for no-arg TextDecoder (Response/Blob text path)
new TextDecoder() with no argument (what Response.text()/Blob.text() and most
pages use) is UTF-8; resolve that without the op_encoding_for_label round trip.
Explicit labels still validate via the op.
* core: HTMLDialogElement, Node.normalize, Range.createContextualFragment
HTMLDialogElement was a bare alias of Element, so ~40 Core files crashed at the
first dialog.show()/showModal(). Add the dialog members on Element.prototype
(matching the popover/input convention): open reflection, returnValue (internal
slot, String coercion), oncancel/onclose defaulting to null, show()/showModal()
/close()/requestClose() with the open-state transitions, InvalidStateError
throwing (show-on-modal, showModal-on-open, showModal-disconnected), the async
non-bubbling close event, the synchronous cancelable cancel event (no re-entrant
cancel), beforetoggle/toggle, and closedBy. matches(':modal') reports a modally
shown dialog (like :popover-open). Modal top-layer/focus/render stays unhandled
(layout).
Node.normalize() now merges adjacent Text nodes and drops empty ones (was a
no-op). Range.createContextualFragment(html) parses a fragment.
createContextualFragment 34/35, dialog-open 3/3, dialog-return-value 1/1,
dialog-showModal 5/10, dialog-requestclose 22/75. Not on hot paths.
* custom-elements: CustomElementRegistry class, getName, ElementInternals, HTMLUnknownElement
customElements was a bare object literal, so 'CustomElementRegistry is not
defined' crashed ~22 Core files at top level (idlharness/feature-detect), and
HTMLUnknownElement/ElementInternals/attachInternals/getName were missing.
Promote the registry to a real CustomElementRegistry class (window.customElements
is now an instance, so 'define in CustomElementRegistry.prototype' works and the
class is exposed). Add spec define() validation/throw order (TypeError for a
non-constructor, SyntaxError for an invalid name, NotSupportedError for a
duplicate name/constructor or reentrant define), getName(), and a SyntaxError
reject in whenDefined for invalid names. The existing define -> upgrade ->
connectedCallback path (used by lit/Polymer on real pages) is preserved.
Add HTMLUnknownElement, ElementInternals (setFormValue/setValidity/checkValidity/
validity/validationMessage/form/states), and Element.attachInternals() with its
NotSupportedError cases.
Does NOT include the [[HTMLConstructor]] (new.target) construction rework or the
reaction wiring into appendChild/setAttribute (hot paths) - deferred to avoid
element-model and perf risk. getName 4/4.
* shadow-dom: ShadowRoot.adoptedStyleSheets (mirror Document)
ShadowRoot lacked adoptedStyleSheets; constructible-stylesheet adoption into a
shadow root threw. Add the getter/setter on ShadowRoot.prototype, matching
Document.adoptedStyleSheets.
* conformance: charset/URL encoding override, IDL reflection, Range and Storage fixes
Encoding: track the detected document character encoding and expose it as
document.characterSet/charset/inputEncoding instead of hardcoding UTF-8. Add the
WHATWG URL encoding override for <a>/<area> hrefs, so a legacy-charset document
percent-encodes the query in its own encoding (EUC-JP U+8108 becomes %CC%AE).
ASCII delimiters stay literal so structured queries are unaffected, and
unmappable code points become %26%23NNN%3B. UTF-8 documents take an early-return
so the href getter hot path is unchanged. fetch()/XHR text() now decode using
the response Content-Type charset instead of always UTF-8. Adds op_url_encode_query
and a document_encoding op; encoding state is plumbed through Page and the runtime.
Reflection: install the reflected IDL attribute layer once on Element.prototype
(no per-element defineProperty, so element creation and query paths are
untouched): global attributes (title, lang, dir, accessKey, slot, autofocus,
hidden, tabIndex) with correct type coercion and enum canonicalization, plus the
ARIAMixin (aria-* reflected as the ariaXxx nullable DOMString properties).
Range: compareBoundaryPoints ToUint16-converts the how argument (NaN/Infinity
no longer false-throw) and raises a real WrongDocumentError DOMException for
ranges that do not share a root.
Storage: reimplement localStorage/sessionStorage as a WHATWG Proxy so property
access (localStorage.foo = x, delete, in, Object.keys) stays in sync with
length, key() and iteration.
* conformance: HTML attribute folding, compareDocumentPosition, URLSearchParams, DOMParser, fetch UA
Next WPT Core batch, all verified live and benchmarked for no hot-path regression.
DOM:
- compareDocumentPosition rewritten to spec: real PRECEDING/FOLLOWING via the
compare_order op, CONTAINS/CONTAINED_BY, and DISCONNECTED|IMPLEMENTATION_SPECIFIC
for different roots. Adds the DOCUMENT_POSITION_* and missing nodeType constants
on Node and its prototype. WPT getPosition() depends on this, so it cascades into
the Range comparePoint/compareBoundaryPoints/isPointInRange expectations.
- HTML elements now ASCII-lowercase attribute names in getAttribute/setAttribute/
removeAttribute/hasAttribute, so el.setAttribute('accessKey') reflects via
getAttribute('accesskey'). The fold is gated behind a cheap uppercase scan and a
namespaceURI HTML guard (SVG/foreign attributes keep their case), and getAttribute
only folds on a miss, so the all-lowercase hot path is unchanged (benchmarked flat).
- Range comparePoint/isPointInRange ToUint32-coerce the offset (negative throws
IndexSizeError); createTreeWalker/createNodeIterator keep an explicit whatToShow of 0.
URL:
- URLSearchParams uses a proper form-urlencoded byte decoder (partial/invalid escapes
stay literal instead of decodeURIComponent throwing on the whole string), 2-arg
has(name,value)/delete(name,value), and a sequence-pair TypeError. Adds URL.parse
and URL.canParse statics.
Other:
- fetch()/XHR send a default Chrome User-Agent (the navigation path set one but
scripted requests went out with none, so UA-gated servers rejected them); honors an
explicit override.
- DOMParser documents expose URL/compatMode/characterSet/readyState/styleSheets/
defaultView and createTreeWalker/createNodeIterator; XMLDocument is defined globally.
* perf(dom): O(1) querySelector('#id') via the id index
querySelector('#id') went through the full selector engine, scanning descendants
in tree order until the first match (O(n), ~68us per call over a 2000-element
document in the micro-bench). A bare ASCII id selector now resolves through the
existing id index in O(1): ~68us -> ~1.25us per call, about 55x faster, with no
change to any other path (create/getAttribute/setAttribute/querySelectorAll/href
all flat). querySelector by id is one of the hottest scraping operations.
The fast path is conservative: only a bare `#name` selector with an ASCII
letter/underscore-led identifier takes it; escapes, combinators, commas,
whitespace, compounds, and non-ASCII fall through to the full engine unchanged.
For a scoped element.querySelector it returns the indexed element only when it is
a strict descendant of the scope, and falls back to the scan otherwise, so
duplicate-id scoped queries stay correct.
Also makes the id index keep the FIRST element in tree order per id (or_insert at
parse time) instead of the last, so getElementById and querySelector('#id') return
the first-in-tree-order element on duplicate ids, per spec (previously last).
* fix: bound synchronous V8 hangs with a termination watchdog and add op panic safety
Heavy pages could hang obscura fetch indefinitely: tokio::time::timeout only
cancels at await points, so synchronous V8 work (a page's script bootstrap, a
microtask storm) pinned the thread and ran unbounded.
Add a reusable V8 terminate_execution watchdog (arm_watchdog/disarm_watchdog) and
apply it to every place that runs synchronous V8 work: the post-load settle, both
navigation event-loop pumps, the whole script-execution phase, and --eval. A CLI
process-level hard deadline backstops the rest, since a hang inside a Rust op
cannot be cancelled by tokio or terminate_execution.
Also harden against panics: op_dom is wrapped in catch_unwind so a DOM-op panic
degrades to a null result instead of unwinding into V8's FFI frame and aborting
the process; the append_text unwrap is removed; panic = unwind is pinned in the
release profile.
Verified: obstacle course 33/33, no perf regression (per-op deltas within the
measurement noise floor), 6 of 8 previously-hanging real sites now render and the
other 2 exit deterministically at the hard deadline.
* fix(dom): reject cyclic reparenting so descendants() cannot hang the engine
appendChild/insertBefore only guarded the self-append case (parent == child),
never the case where the moved node is an ancestor of the new parent. When page
JS does node.appendChild(ancestorOfNode) the parent/child graph becomes cyclic;
the next querySelector runs descendants() (a DFS with no visited set) and loops
forever, growing a Vec unbounded and pinning the thread in native Rust where
neither tokio nor the V8 watchdog can interrupt it. This hung techcrunch and
kayak.
Reject the cycle at the mutation site per the DOM HierarchyRequestError rule: if
the moved node is an inclusive ancestor of its target parent, treat the move as a
no-op. The walk is gated on the moved node having children, so a fresh/leaf
append (the common case) stays O(1); only relocating a populated subtree pays
O(depth). descendants() also gets a result-length cap as defense in depth so a
cycle from any future path cannot wedge the engine.
techcrunch and kayak now render (37K and 1.26M chars). Verified: new
test_reparent_cycle_is_rejected, obstacle course 33/33, no perf regression.
This commit is contained in:
SGavrl authored and GitHub committed
1 parent
2e731a2ab9
commit
1e9e9478fb
6 files changed
+335
-12
No files matched your search
@@ -43,3 +43,10 @@ uuid = { version = "1", features = ["v4"] }
|
||||
thiserror = "2"
|
||||
anyhow = "1"
|
||||
base64 = "0.22"
|
||||
|
||||
# Unwinding (the Rust default) is REQUIRED for the anti-panic protocol: ops wrap
|
||||
# their bodies in std::panic::catch_unwind so a panic degrades to an error return
|
||||
# instead of unwinding into V8's FFI frame and aborting the process. panic="abort"
|
||||
# would turn every catchable op panic into a hard crash.
|
||||
[profile.release]
|
||||
panic = "unwind"
|
||||
@@ -295,6 +295,17 @@ impl Page {
|
||||
let script_deadline = tokio::time::Instant::now()
|
||||
+ tokio::time::Duration::from_millis(script_deadline_ms);
|
||||
|
||||
// Hard backstop over the WHOLE script-execution phase. Inline scripts
|
||||
// run back-to-back with no await between them, so neither the soft
|
||||
// deadline above (only checked between scripts) nor the per-script guard
|
||||
// can interrupt a page that burns the budget across many synchronous
|
||||
// scripts (the real-world SPA / anti-bot busy-loop hang). This watchdog
|
||||
// terminates the isolate if cumulative synchronous script work overruns.
|
||||
let exec_wd = self
|
||||
.js
|
||||
.as_mut()
|
||||
.map(|js| js.arm_watchdog(std::time::Duration::from_millis(script_deadline_ms + 1000)));
|
||||
|
||||
#[derive(Debug)]
|
||||
struct ScriptInfo {
|
||||
src: Option<String>,
|
||||
@@ -596,6 +607,10 @@ impl Page {
|
||||
// On busy sites this could keep the V8 lock held for tens of
|
||||
// seconds, wedging the entire CDP dispatcher (see triage for
|
||||
// issue series around the 40-site compat sweep).
|
||||
// A single run_event_loop poll that pins the thread inside V8 makes
|
||||
// the per-poll tokio timeouts below useless, so guard the whole loop
|
||||
// with a watchdog that fires ~250ms past its 500ms deadline.
|
||||
let settle_wd = js.arm_watchdog(std::time::Duration::from_millis(750));
|
||||
let deadline = tokio::time::Instant::now() + tokio::time::Duration::from_millis(500);
|
||||
let mut idle_count = 0u32;
|
||||
loop {
|
||||
@@ -626,6 +641,12 @@ impl Page {
|
||||
}
|
||||
}
|
||||
}
|
||||
js.disarm_watchdog(settle_wd);
|
||||
}
|
||||
if let Some(token) = exec_wd {
|
||||
if let Some(js) = self.js.as_mut() {
|
||||
js.disarm_watchdog(token);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -694,11 +715,11 @@ impl Page {
|
||||
return;
|
||||
}
|
||||
if let Some(js) = &mut self.js {
|
||||
let _ = tokio::time::timeout(
|
||||
tokio::time::Duration::from_millis(max_ms),
|
||||
js.run_event_loop(),
|
||||
)
|
||||
.await;
|
||||
// Bounded against both async idle and synchronous microtask storms:
|
||||
// a plain tokio timeout cannot preempt a page that pins the thread
|
||||
// inside V8 (the real-world SPA hang), so settle drives the loop
|
||||
// through the watchdog-guarded path.
|
||||
let _ = js.run_event_loop_bounded(max_ms).await;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1010,6 +1031,13 @@ impl Page {
|
||||
_ => 0,
|
||||
};
|
||||
|
||||
// Same hazard as the post-script settle: a synchronous poll can pin
|
||||
// the thread past the 5s network-idle deadline, so arm a watchdog
|
||||
// that terminates the isolate ~500ms past it.
|
||||
let netidle_wd = self
|
||||
.js
|
||||
.as_mut()
|
||||
.map(|js| js.arm_watchdog(std::time::Duration::from_millis(5500)));
|
||||
let deadline = tokio::time::Instant::now() + tokio::time::Duration::from_secs(5);
|
||||
let mut idle_since: Option<tokio::time::Instant> = None;
|
||||
|
||||
@@ -1043,6 +1071,11 @@ impl Page {
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(token) = netidle_wd {
|
||||
if let Some(js) = self.js.as_mut() {
|
||||
js.disarm_watchdog(token);
|
||||
}
|
||||
}
|
||||
self.lifecycle = LifecycleState::NetworkIdle;
|
||||
}
|
||||
|
||||
@@ -1075,6 +1108,27 @@ impl Page {
|
||||
self.dom.as_ref()
|
||||
}
|
||||
|
||||
/// Like [`Self::evaluate`] but bounded by a V8 watchdog so a runaway
|
||||
/// expression cannot hang the process. A non-zero `timeout` of zero falls
|
||||
/// back to the unbounded path.
|
||||
pub fn evaluate_with_timeout(
|
||||
&mut self,
|
||||
expression: &str,
|
||||
timeout: std::time::Duration,
|
||||
) -> serde_json::Value {
|
||||
if let Some(js) = &mut self.js {
|
||||
match js.evaluate_with_timeout(expression, timeout) {
|
||||
Ok(val) => val,
|
||||
Err(e) => {
|
||||
tracing::debug!("JS eval error/timeout for '{}': {}", &expression[..expression.len().min(80)], e);
|
||||
serde_json::Value::Null
|
||||
}
|
||||
}
|
||||
} else {
|
||||
self.evaluate(expression)
|
||||
}
|
||||
}
|
||||
|
||||
pub fn evaluate(&mut self, expression: &str) -> serde_json::Value {
|
||||
if let Some(js) = &mut self.js {
|
||||
match js.evaluate(expression) {
|
||||
|
||||
@@ -513,6 +513,22 @@ async fn run_fetch(
|
||||
eprintln!("Fetching {}...", url_str);
|
||||
}
|
||||
|
||||
// Process-level hard deadline. A synchronous hang inside a Rust op invoked
|
||||
// from page JS cannot be cancelled by tokio (there is no await to interrupt)
|
||||
// nor by the V8 watchdog (terminate_execution only unwinds JS bytecode, not
|
||||
// native Rust running beneath a V8->op call). As an absolute backstop so one
|
||||
// fetch can never wedge the worker, a daemon thread force-exits if the whole
|
||||
// operation overruns timeout + wait + grace. A normal fetch returns first and
|
||||
// the process exits before this fires.
|
||||
{
|
||||
let hard = Duration::from_secs(timeout_secs.saturating_add(wait_secs).saturating_add(10));
|
||||
std::thread::spawn(move || {
|
||||
std::thread::sleep(hard);
|
||||
eprintln!("obscura: hard timeout exceeded ({}s); forcing exit", hard.as_secs());
|
||||
std::process::exit(124);
|
||||
});
|
||||
}
|
||||
|
||||
match timeout(Duration::from_secs(timeout_secs), page.navigate_with_wait(url_str, wait_condition)).await {
|
||||
Ok(result) => result.map_err(|e| anyhow::anyhow!("Failed to navigate to {}: {}", url_str, e))?,
|
||||
Err(_) => anyhow::bail!(
|
||||
@@ -540,7 +556,9 @@ async fn run_fetch(
|
||||
}
|
||||
|
||||
if let Some(ref expr) = eval {
|
||||
let result = page.evaluate(expr);
|
||||
// Bound the eval by the same budget as navigation so a runaway
|
||||
// expression (infinite loop, never-settling sync work) cannot hang.
|
||||
let result = page.evaluate_with_timeout(expr, Duration::from_secs(timeout_secs));
|
||||
let rendered = match result {
|
||||
serde_json::Value::String(s) => s,
|
||||
serde_json::Value::Null => "null".to_string(),
|
||||
|
||||
@@ -234,6 +234,40 @@ impl DomTree {
|
||||
if parent_id == child_id {
|
||||
return;
|
||||
}
|
||||
// Appending an ancestor of the parent under that parent makes the
|
||||
// parent/child graph cyclic, and every later descendants()/children()/
|
||||
// textContent walk (none carry a visited set) would loop forever, pinning
|
||||
// the thread in native Rust where neither tokio nor the V8 watchdog can
|
||||
// interrupt it. Per the DOM spec this is a HierarchyRequestError; treat it
|
||||
// as a no-op, like the self-append guard above. Only a node that already
|
||||
// has children can be an ancestor, so a fresh/leaf child (the common
|
||||
// append) skips the walk: O(1) hot path, O(depth) only when relocating a
|
||||
// populated subtree.
|
||||
{
|
||||
let inner = self.inner.borrow();
|
||||
let child_has_children = inner.nodes.get(child_id.index())
|
||||
.and_then(|n| n.as_ref())
|
||||
.map(|n| n.first_child.is_some())
|
||||
.unwrap_or(false);
|
||||
if child_has_children {
|
||||
let mut cur = inner.nodes.get(parent_id.index())
|
||||
.and_then(|n| n.as_ref())
|
||||
.and_then(|n| n.parent);
|
||||
let mut steps = 0usize;
|
||||
while let Some(p) = cur {
|
||||
if p == child_id {
|
||||
return;
|
||||
}
|
||||
steps += 1;
|
||||
if steps > inner.nodes.len() {
|
||||
return; // pre-existing corruption: refuse rather than risk a cycle
|
||||
}
|
||||
cur = inner.nodes.get(p.index())
|
||||
.and_then(|n| n.as_ref())
|
||||
.and_then(|n| n.parent);
|
||||
}
|
||||
}
|
||||
}
|
||||
self.detach(child_id);
|
||||
|
||||
let mut inner = self.inner.borrow_mut();
|
||||
@@ -284,6 +318,35 @@ impl DomTree {
|
||||
}
|
||||
};
|
||||
|
||||
// Inserting the parent itself, or any ancestor of the parent, as a child
|
||||
// of that parent would create a cycle (same non-terminating-walk hang as
|
||||
// append_child). Reject it, matching the self-insert guard above. Gate on
|
||||
// the inserted node actually having children, so the common case (insert a
|
||||
// fresh node) stays O(1).
|
||||
{
|
||||
let inner = self.inner.borrow();
|
||||
let new_has_children = inner.nodes.get(new_sibling_id.index())
|
||||
.and_then(|n| n.as_ref())
|
||||
.map(|n| n.first_child.is_some())
|
||||
.unwrap_or(false);
|
||||
if new_has_children {
|
||||
let mut cur = Some(parent_id);
|
||||
let mut steps = 0usize;
|
||||
while let Some(p) = cur {
|
||||
if p == new_sibling_id {
|
||||
return;
|
||||
}
|
||||
steps += 1;
|
||||
if steps > inner.nodes.len() {
|
||||
return;
|
||||
}
|
||||
cur = inner.nodes.get(p.index())
|
||||
.and_then(|n| n.as_ref())
|
||||
.and_then(|n| n.parent);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
self.detach(new_sibling_id);
|
||||
|
||||
let mut inner = self.inner.borrow_mut();
|
||||
@@ -443,6 +506,14 @@ impl DomTree {
|
||||
|
||||
while let Some(current) = stack.pop() {
|
||||
result.push(current);
|
||||
// Defense in depth: a well-formed subtree has at most nodes.len()
|
||||
// descendants. Exceeding that means the parent/child graph is cyclic
|
||||
// (which the append_child / insert_before guards prevent); stop rather
|
||||
// than grow the stack and result forever and wedge the engine. On a
|
||||
// valid tree this bound is never reached, so the hot path is unchanged.
|
||||
if result.len() > inner.nodes.len() {
|
||||
break;
|
||||
}
|
||||
|
||||
let mut child = inner.nodes.get(current.index())
|
||||
.and_then(|n| n.as_ref())
|
||||
@@ -515,18 +586,22 @@ impl DomTree {
|
||||
};
|
||||
|
||||
if last_child_is_text {
|
||||
// Re-read last_child without unwrap: if it vanished between the two
|
||||
// borrows, fall through to appending a fresh text node rather than
|
||||
// panicking (a panic here aborts the whole engine via V8_Fatal).
|
||||
let last_child_id = {
|
||||
let inner = self.inner.borrow();
|
||||
inner.nodes.get(parent_id.index())
|
||||
.and_then(|n| n.as_ref())
|
||||
.and_then(|n| n.last_child)
|
||||
.unwrap()
|
||||
};
|
||||
let mut inner = self.inner.borrow_mut();
|
||||
if let Some(Some(node)) = inner.nodes.get_mut(last_child_id.index()) {
|
||||
if let NodeData::Text { contents } = &mut node.data {
|
||||
contents.push_str(text);
|
||||
return;
|
||||
if let Some(last_child_id) = last_child_id {
|
||||
let mut inner = self.inner.borrow_mut();
|
||||
if let Some(Some(node)) = inner.nodes.get_mut(last_child_id.index()) {
|
||||
if let NodeData::Text { contents } = &mut node.data {
|
||||
contents.push_str(text);
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -737,6 +812,47 @@ mod tests {
|
||||
assert_eq!(tree.get_element_by_id("nonexistent"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_reparent_cycle_is_rejected() {
|
||||
// document -> html -> body -> div. Moving an ancestor under one of its
|
||||
// own descendants would make the parent/child graph cyclic and hang
|
||||
// every later descendants() walk. Both append_child and insert_before
|
||||
// must reject it as a no-op (DOM HierarchyRequestError).
|
||||
let tree = DomTree::new();
|
||||
let doc = tree.document();
|
||||
let mk = |n: &str| {
|
||||
tree.new_node(NodeData::Element {
|
||||
name: QualName::new(None, ns!(html), LocalName::from(n)),
|
||||
attrs: vec![],
|
||||
template_contents: None,
|
||||
mathml_annotation_xml_integration_point: false,
|
||||
})
|
||||
};
|
||||
let html = mk("html");
|
||||
let body = mk("body");
|
||||
let div = mk("div");
|
||||
tree.append_child(doc, html);
|
||||
tree.append_child(html, body);
|
||||
tree.append_child(body, div);
|
||||
|
||||
let before = tree.descendants(doc).len();
|
||||
assert_eq!(before, 3);
|
||||
|
||||
// append_child: html is an ancestor of div -> must be a no-op, no cycle.
|
||||
tree.append_child(div, html);
|
||||
assert_eq!(tree.descendants(doc).len(), before, "cyclic append must be a no-op");
|
||||
assert_eq!(tree.descendants(div).len(), 0, "div must stay a leaf");
|
||||
|
||||
// insert_before: html is an ancestor of body (div's parent) -> no-op.
|
||||
tree.insert_before(div, html);
|
||||
assert_eq!(tree.descendants(doc).len(), before, "cyclic insert_before must be a no-op");
|
||||
|
||||
// self-append / self-insert remain no-ops (existing guards).
|
||||
tree.append_child(div, div);
|
||||
tree.insert_before(div, div);
|
||||
assert_eq!(tree.descendants(doc).len(), before);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_append_text_merges() {
|
||||
let tree = DomTree::new();
|
||||
|
||||
@@ -83,6 +83,22 @@ pub type SharedState = Rc<RefCell<ObscuraState>>;
|
||||
#[op2]
|
||||
#[string]
|
||||
fn op_dom(state: &OpState, #[string] cmd: String, #[string] arg1: String, #[string] arg2: String) -> String {
|
||||
// Anti-panic boundary: a panic in a DOM op would unwind through deno_core
|
||||
// into V8's FFI frame, where V8_Fatal calls abort(3) and takes the whole
|
||||
// engine (and every CDP client) down. Catch it so one malformed selector or
|
||||
// inconsistent tree node degrades to a null result for that single call.
|
||||
// No per-call clone: on the happy path this is just a landing pad, so the
|
||||
// hot DOM path (querySelector/getAttribute/...) pays nothing measurable.
|
||||
std::panic::catch_unwind(std::panic::AssertUnwindSafe(move || {
|
||||
op_dom_inner(state, cmd, arg1, arg2)
|
||||
}))
|
||||
.unwrap_or_else(|_| {
|
||||
tracing::error!("op_dom panicked; returning null");
|
||||
"null".to_string()
|
||||
})
|
||||
}
|
||||
|
||||
fn op_dom_inner(state: &OpState, cmd: String, arg1: String, arg2: String) -> String {
|
||||
let gs = state.borrow::<SharedState>().clone();
|
||||
let gs = gs.borrow();
|
||||
let dom = match &gs.dom {
|
||||
|
||||
@@ -27,6 +27,15 @@ pub struct ObscuraJsRuntime {
|
||||
object_counter: u64,
|
||||
}
|
||||
|
||||
/// Handle to an armed V8 execution watchdog (see [`ObscuraJsRuntime::arm_watchdog`]).
|
||||
/// Holds the cancel channel and the watchdog thread; pass it back to
|
||||
/// `disarm_watchdog` to stop the watchdog and learn whether it fired.
|
||||
pub struct WatchdogToken {
|
||||
pair: std::sync::Arc<(std::sync::Mutex<bool>, std::sync::Condvar)>,
|
||||
join: Option<std::thread::JoinHandle<()>>,
|
||||
fired: std::sync::Arc<std::sync::atomic::AtomicBool>,
|
||||
}
|
||||
|
||||
impl ObscuraJsRuntime {
|
||||
pub fn new() -> Self {
|
||||
Self::with_base_url("about:blank")
|
||||
@@ -671,6 +680,109 @@ impl ObscuraJsRuntime {
|
||||
.map_err(|e| format!("Event loop error: {}", e))
|
||||
}
|
||||
|
||||
/// Arm a hard wall-clock backstop on synchronous V8 work. A page stuck in a
|
||||
/// synchronous loop or a microtask storm pins the OS thread inside V8, so
|
||||
/// `tokio::time::timeout` (which can only cancel at await points) never
|
||||
/// fires. This spawns a watchdog thread that terminates the isolate once
|
||||
/// `budget` elapses, forcing V8 to throw an uncatchable error and hand
|
||||
/// control back. Always balance with [`Self::disarm_watchdog`].
|
||||
pub fn arm_watchdog(&mut self, budget: std::time::Duration) -> WatchdogToken {
|
||||
let handle = self.runtime.v8_isolate().thread_safe_handle();
|
||||
let pair = std::sync::Arc::new((std::sync::Mutex::new(false), std::sync::Condvar::new()));
|
||||
let fired = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
|
||||
let pair_c = pair.clone();
|
||||
let fired_c = fired.clone();
|
||||
let join = std::thread::spawn(move || {
|
||||
let (lock, cvar) = &*pair_c;
|
||||
let mut cancelled = lock.lock().unwrap();
|
||||
let deadline = std::time::Instant::now() + budget;
|
||||
loop {
|
||||
let remaining = deadline.saturating_duration_since(std::time::Instant::now());
|
||||
if remaining.is_zero() {
|
||||
fired_c.store(true, std::sync::atomic::Ordering::SeqCst);
|
||||
handle.terminate_execution();
|
||||
return;
|
||||
}
|
||||
let (guard, _) = cvar.wait_timeout(cancelled, remaining).unwrap();
|
||||
cancelled = guard;
|
||||
if *cancelled {
|
||||
return;
|
||||
}
|
||||
}
|
||||
});
|
||||
WatchdogToken { pair, join: Some(join), fired }
|
||||
}
|
||||
|
||||
/// Stop a watchdog armed by [`Self::arm_watchdog`]. If it had already fired
|
||||
/// (terminated the isolate), clear V8's termination flag so the isolate is
|
||||
/// usable again, and return `true`.
|
||||
pub fn disarm_watchdog(&mut self, mut token: WatchdogToken) -> bool {
|
||||
{
|
||||
let (lock, cvar) = &*token.pair;
|
||||
*lock.lock().unwrap() = true;
|
||||
cvar.notify_one();
|
||||
}
|
||||
if let Some(j) = token.join.take() {
|
||||
let _ = j.join();
|
||||
}
|
||||
let fired = token.fired.load(std::sync::atomic::Ordering::SeqCst);
|
||||
if fired {
|
||||
self.runtime.v8_isolate().cancel_terminate_execution();
|
||||
tracing::warn!("V8 watchdog fired: terminated a synchronous overrun");
|
||||
}
|
||||
fired
|
||||
}
|
||||
|
||||
/// Drive the event loop for at most `budget_ms`, bounded against BOTH async
|
||||
/// idle (tokio timeout) and synchronous hangs (V8 watchdog). A microtask
|
||||
/// storm that pins the thread is terminated ~500ms past the budget; a
|
||||
/// well-behaved page returns as soon as the loop goes idle.
|
||||
pub async fn run_event_loop_bounded(&mut self, budget_ms: u64) -> Result<(), String> {
|
||||
if budget_ms == 0 {
|
||||
return self.run_event_loop().await;
|
||||
}
|
||||
let budget = std::time::Duration::from_millis(budget_ms);
|
||||
let token = self.arm_watchdog(budget + std::time::Duration::from_millis(500));
|
||||
let result = tokio::time::timeout(budget, self.run_event_loop()).await;
|
||||
self.disarm_watchdog(token);
|
||||
match result {
|
||||
Ok(Ok(())) => Ok(()),
|
||||
Ok(Err(e)) if e.contains("execution terminated") => Ok(()),
|
||||
Ok(Err(e)) => Err(e),
|
||||
// tokio idle-timeout is the normal "settled" exit, not an error.
|
||||
Err(_) => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Like [`Self::evaluate`] but bounded by a V8 watchdog, so a `--eval`
|
||||
/// expression that loops forever (or awaits a promise that never settles in
|
||||
/// synchronous form) cannot hang the process.
|
||||
pub fn evaluate_with_timeout(
|
||||
&mut self,
|
||||
expression: &str,
|
||||
timeout: std::time::Duration,
|
||||
) -> Result<serde_json::Value, String> {
|
||||
if timeout.is_zero() {
|
||||
return self.evaluate(expression);
|
||||
}
|
||||
let wrapped = Self::wrap_expression(expression);
|
||||
let token = self.arm_watchdog(timeout);
|
||||
let result = self.runtime.execute_script("<eval>", wrapped);
|
||||
let fired = self.disarm_watchdog(token);
|
||||
match result {
|
||||
Ok(v) if !fired => self.v8_to_json(v),
|
||||
Ok(_) => Err("eval timed out".to_string()),
|
||||
Err(e) => {
|
||||
let msg = e.to_string();
|
||||
if fired || msg.contains("execution terminated") {
|
||||
Err("eval timed out".to_string())
|
||||
} else {
|
||||
Err(format!("JS error: {}", msg))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn resolve_promises(&mut self) {
|
||||
// Default settle: just pump until idle or 5s.
|
||||
let _ = tokio::time::timeout(
|
||||
|
||||
Reference in new issue
Block a user