mirror of
https://github.com/daijro/camoufox.git
synced 2026-09-08 08:00:53 +00:00
Jake Writer
4132dd50a6
Four deadlocks shipped between 2026-04 and 2026-09 -- exact-edge coordinates (9270618), humanized trajectory points that bypassed the endpoint's guard (541ffca, #225/#677), a zero-displacement move (16e5a13), and the near edge (014cc65, #751/#752). Each was fixed by adding one more coordinate guard at one more call site. That does not converge, for two reasons. The trigger set is not enumerable. Whether relative y == 0 reaches the renderer is decided by Math.round(boundingBox.top) < boundingBox.top -- a rounding accident in the fractional height of browser chrome, which varies with the spoofed OS. No review catches that. And every miss costs the whole process. activateAndRun() serializes input on a chain shared by every tab; EventWatcher.ensureEvent() waited forever. One missing ack wedged every later input event in the process, permanently, at 0% CPU with no diagnostic. #677 shows why review is not the answer: restoring the humanize trajectory meant writing a bounds check, and the one written was a copy of the pre-#225 form, reintroducing a fixed deadlock one day before it was re-fixed. Three changes, in order of leverage. 1. Bound the waits. EventWatcher.ensureEventWithin() gives up instead of waiting forever; MouseDispatch.sendAcked() uses it, drops the event and logs the type, coordinate and browser rect. This alone closes all four historical deadlocks, including on a build with no coordinate fix at all. The 5s deadline is sized from measurement, not intuition. Over 1000+ dispatches: idle content thread p50 0ms / p99 1ms / max 12ms; a thread burning 8ms per event p50 8ms / max 12ms. But the ack is delivered FROM the content main thread, so a page running a 3s synchronous script delayed a legitimate ack by 2849ms. Block length is page-controlled and unbounded, and silently dropping real input on a slow page is the #752 symptom, so the deadline sits above the slowest legitimate ack rather than near the typical one. Bounding each ack is not enough to bound the work: a humanized curve is ~110 points in a single activation-chain slot. sendTrajectoryAcked() abandons the rest of a curve after the first undelivered point -- not a wall-clock budget, which would false-fire on exactly the slow pages the deadline exists to tolerate. activateAndRun() carries a 30s backstop for the other unbounded waits reachable from the same slot (apz-repaints-flushed, TabSwitchDone, the drag path's waits), none of which has failed yet. 2. One chokepoint. additions/juggler/input/MouseDispatch.js owns the relative-to-absolute conversion, the in-viewport predicate and the ack wait. PageHandler's three independent bounds checks and its raw jugglerSendMouseEvent/sendWheelEvent calls are gone; it now passes relative coordinates and never sees a bounding box. Net effect on that vendored file is 92 lines removed against 22 added -- a smaller diff against upstream juggler, since the logic moved into a file we own. Wheel events go through the same conversion, so a wheel at relative y == 0 no longer scrolls the tab strip. 3. Enforcement. scripts/check-input-dispatch.py fails the build if anything outside the chokepoint dispatches synthesized input or does browser-relative coordinate arithmetic. It needs no browser build, so .github/workflows/lint.yml gates every pull request -- nothing was checking PRs before. Two exemptions, both content-process: PageAgent (drag events, already content-relative, no ack) and FrameTree (the ack producer). docs/input-dispatch.md states the invariant. tests/patches/mouse-boundary-sweep.py replaces hand-picked edge targets, which are what let each of the four through: humanize-edge-deadlock.py probes only the far edges, and humanize-mouse-trajectory.py pins os="linux" -- the one fingerprint immune to #751. It sweeps the whole viewport ring across every spoofed OS with humanize on and off, asserting each point is acked AND observed by the page. It depends on change 1 to run at all: without the backstop the first bad coordinate wedges the browser and the sweep dies there. tests/patches/input-ack-backstop.py covers the bounded wait itself, by blocking the content main thread far longer than the deadline -- a legitimate late ack, with no test-only hook in production code. The sweep immediately found a fifth instance, pre-existing and unreported: boundingBox.height is consistently 0.5 CSS px less than the innerHeight the page reports, so the page's last row is half covered. With the box at 1920x977.5 +0+56.5, relative y == 977 -- innerHeight - 1, well inside the viewport as far as the page is concerned -- dispatches at 1033.5, rounds to 1034, and the content ends at 1034. Deterministic, 4/4, and it deadlocks a stock build. Fixed by clamping to the last whole pixel inside the element, symmetric with the near-edge snap; both live in the one conversion now. All seven patch tests pass: mouse-boundary-sweep (150 ring coordinates over 6 scenarios), near-edge-mouse-deadlock, input-ack-backstop, humanize-edge-deadlock, humanize-mouse-trajectory, noop-mousemove-deadlock, trusted-events. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GQgHHGRXNp29jr4xQjK7iv (cherry picked from commit827b98d31e)
74 lines
3.7 KiB
Markdown
74 lines
3.7 KiB
Markdown
# Synthesized input dispatch
|
|
|
|
Every synthesized mouse and wheel event in the parent process goes through
|
|
`additions/juggler/input/MouseDispatch.js`. `scripts/check-input-dispatch.py`
|
|
fails the build if anything else dispatches input or does browser-relative
|
|
coordinate arithmetic, and it runs on every pull request.
|
|
|
|
## The invariant
|
|
|
|
> A synthesized input event whose ack we await must reach the content
|
|
> renderer — and when it does not, we must stop waiting.
|
|
|
|
## Why it is worth a module and a lint
|
|
|
|
Four deadlocks shipped between 2026-04 and 2026-09, all the same failure:
|
|
|
|
| Date | Commit | Trigger |
|
|
|---|---|---|
|
|
| 2026-06-04 | `9270618` | `x == width` / `y == height` — the far edges |
|
|
| 2026-07-18 | `541ffca` | trajectory points, which bypassed the endpoint's guard (#225, #677) |
|
|
| 2026-07-24 | `16e5a13` | a zero-displacement move |
|
|
| 2026-09-03 | `014cc65` | `y == 0` — the near edge (#751, #752) |
|
|
|
|
Each was fixed by adding one more coordinate guard at one more call site. That
|
|
does not converge, for two reasons.
|
|
|
|
**The trigger set is not enumerable.** Whether relative `y == 0` reaches the
|
|
renderer is decided by `Math.round(boundingBox.top) < boundingBox.top` — a
|
|
rounding accident in the fractional height of browser chrome, which varies with
|
|
the *spoofed OS*: windows `51.4` → `51` deadlocks, macos `53.1` → `53`
|
|
deadlocks, linux `56.5` → `57` is fine. No review catches that, and no
|
|
hand-written list of coordinates contains it.
|
|
|
|
**Every miss costs the whole process.** `activateAndRun()`
|
|
(`TargetRegistry.js`) serializes input on a promise chain shared by every tab in
|
|
the process. It swallows errors to keep the chain running, but it cannot swallow
|
|
a callback that never returns. One unbounded `await` for an ack that will never
|
|
arrive wedges every later input event, in every tab, permanently — at 0% CPU,
|
|
with nothing in flight and no diagnostic.
|
|
|
|
`#677` is why review is not enough: restoring the humanize trajectory meant
|
|
writing a bounds check, and the one written was a copy of the pre-`#225` form,
|
|
reintroducing a fixed deadlock one day before it was re-fixed.
|
|
|
|
## How it is enforced
|
|
|
|
**One chokepoint.** `MouseDispatch` owns the relative→absolute conversion (with
|
|
the boundary snap), the in-viewport predicate, and the ack wait. Callers pass
|
|
relative coordinates and never see a bounding box.
|
|
|
|
**Bounded waits.** `sendAcked()` waits at most `kAckDeadlineMs` (5s) and then
|
|
drops the event with a warning naming the type, coordinate and browser rect. The
|
|
deadline is sized above the slowest *legitimate* ack, not near the typical one:
|
|
acks are p99 1ms on an idle page, but they are delivered from the content main
|
|
thread and inherit any block on it — a 3s synchronous script delayed one by
|
|
2849ms. `sendTrajectoryAcked()` abandons the rest of a curve after the first
|
|
undelivered point, so ~110 bounded waits cannot add up to an unbounded slot.
|
|
`activateAndRun()` carries a 30s backstop for the other unbounded waits
|
|
reachable from the same slot (`apz-repaints-flushed`, `TabSwitchDone`, the drag
|
|
path's waits), none of which has failed yet.
|
|
|
|
**The static check.** `scripts/check-input-dispatch.py`, wired into
|
|
`.github/workflows/lint.yml`. Two exemptions, both content-process:
|
|
`PageAgent.js` (drag events, already content-relative, no ack) and
|
|
`FrameTree.js` (the ack *producer*).
|
|
|
|
**Boundary coverage.** `tests/patches/mouse-boundary-sweep.py` sweeps the whole
|
|
viewport ring across every spoofed OS with humanize on and off, asserting each
|
|
point is acked *and observed by the page*. Hand-picked coordinate lists are what
|
|
let each of the four bugs through: `humanize-edge-deadlock.py` probed only the
|
|
far edges, and `humanize-mouse-trajectory.py` pins `os="linux"` — the one
|
|
fingerprint immune to `#751`. `tests/patches/input-ack-backstop.py` covers the
|
|
bounded wait itself.
|