Skip to content

fix(scene): a browser page that crashes while loading fails at once - #50

Merged
kixelated merged 7 commits into
mainfrom
quest/m0/browser-first-crash
Nov 5, 2026
Merged

kixelated merged 7 commits into
mainfrom
quest/m0/browser-first-crash

Conversation

@kixelated

@kixelated kixelated commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Claims quest/m0/browser-first-crash.md and finishes it.

Summary

A browser source whose renderer dies during its first navigation now fails within milliseconds, as one that crashes later does. a_crash_drops_only_its_source (now a_crash_while_loading_drops_only_its_source) passes on NixOS.

Root cause. The quest's premise, that CEF fires no event for this crash, turned out not to hold. CEF does call on_render_process_terminated, but only once the crashed process is gone. On NixOS, kernel.core_pattern pipes the core to systemd-coredump, and a dying process stays alive until its dump finishes. For the chrome://crash renderer that took 21 seconds in one run and 67 seconds in another (5.9G memory peak, 527MB written; journalctl shows processing... at the crash and dumped core that much later). The test's 20 second budget ran out first. The Ubuntu host where the test passed earlier likely handles the dump faster (apport, or no pipe at all); I did not check that host.

Fix. Before CEF starts, the helper (rs/moq-scene/src/browser/host.rs) sets its soft RLIMIT_CORE to 1. CEF's zygote, renderers, GPU and utility processes inherit it. For a pipe core_pattern the kernel ignores the limit except at exactly 1, where it skips the handler itself (fs/coredump.c coredump_pipe(), logged rate-limited as "RLIMIT_CORE is set to 1, aborting core"); a core file needs at least a page; and systemd-coredump in socket mode declines below a page. So the process exits right away whatever handler the host runs, and CEF reports it. This is the event CEF already gives, with no timeout added. A crash of the helper itself (the browser process) is also reported without waiting on a dump.

Tests. The crash test is split in two (rs/moq-studio/tests/browser.rs):

  • a_crash_while_loading_drops_only_its_source: the previous test, renamed. chrome://crash as the first URL fails only its source; a killed helper fails every page; a reload draws again.
  • a_crash_after_drawing_leaves_the_canvas: a page draws, then the test aborts its renderer (SIGABRT). The source fails with the renderer's reason, leaves the canvas, and a reload draws it again. CEF forks renderers from its zygote, which keeps the zygote's arguments, so the test finds them under the helper by the Compositor thread every renderer runs. It also aborts any spare renderer, which shows no page.

The README's browser paragraph says a crash shows at once, since the browser's processes dump no core.

Choices made without asking

  • No core dumps from CEF's processes. This is the cheapest way to make the existing event arrive at once, and it also spares a live streaming machine 20 to 70 seconds of CPU, memory and disk per crashed page. The cost: a CEF crash leaves no core for debugging. CEF's crash reporter is already off, and the CEF build is a release build without symbols. The studio's own process keeps its limit. Only the soft limit is lowered; the hard limit stays.
  • Limit 1, not 0. Changed in review (OpenAI and pullfrog): 0 works only where the pipe handler honours the %c it is passed, as systemd-coredump does, while Apport still reads the core for a packaged executable and a custom handler need not look at all. 1 makes the kernel skip any pipe handler, at the cost of one rate-limited kernel log line per crash. If the hard limit is 0 the soft limit stays 0, since 1 is forbidden; a pipe handler then dumps unless it honours that 0. host.rs says so.
  • SIGABRT in the post-draw test. kill -SEGV left the renderer running, apparently because V8's WebAssembly trap handler takes a SIGSEGV no fault raised. A page cannot crash itself either: its own location = 'chrome://crash' is silently blocked.

Rejected

  • Waiting longer for the failure (a bigger budget in the test): the delay is real for users too, and no fixed bound covers 70 seconds of dumping.
  • PR_SET_DUMPABLE 0: execve resets it, so each subprocess would need it again, and it also changes /proc ownership and ptrace access.
  • A test-only hook in the helper to crash a page: it would add production protocol for a test, and a signal to the renderer is a real crash.

Validation

On the NixOS host (ssh nixos), in a scratch checkout, with just render-sw's browser setup (lavapipe, xvfb-run):

  • On main: a_crash_drops_only_its_source fails with "renderer never failed" after 20s.
  • With the limit at 0 (head b54720f): all 7 browser tests pass, three runs in a row (9 to 16s per run).
  • With the limit at 1 (head 09e5b25, merged with main): all 7 browser tests pass in three warm runs in a row (2 to 3.5s per run). The kernel log shows "coredump: ...(moq-studio): RLIMIT_CORE is set to 1, aborting core" for each crashed process, and the journal shows no systemd-coredump run. In the first, cold run a_page_draws_and_plays failed once with "the page's tone never reached the bus"; that test crashes nothing, and it passed in the three runs after.
  • With the host.rs change reverted and the new tests kept, a_crash_while_loading_drops_only_its_source fails as on main, and a_crash_after_drawing_leaves_the_canvas passes. A renderer that has already drawn died within a second of SIGABRT even before this change, and the journal shows no dump for it. So the first test guards the fix, and the second covers the crash after commit that the quest asked for.
  • Linux cargo clippy --workspace --all-targets with and without --features moq-scene/capture,moq-studio/capture,moq-studio/openh264,moq-studio/cef and -D warnings: clean. cargo fmt --check: clean.
  • Linux clippy with the features above and -D warnings, and cargo fmt --check, on 09e5b25: clean.
  • just ci on macOS, on head 09e5b25 (after merging main with feat(studio)!: further canvases with their own layouts, drawn from shared sources, each in its own broadcast #42, feat(scene): keyframes that play from when an item's scene goes on #43, feat(scene): a media file source that loops or plays on demand #44): passes, 222 tests.

Review

Codex (quest review), gpt-6.1-sol, medium effort. Round 3 on head a06dd0aa3469f0517b4637525d69f3cf6a3d91c3 reported no findings (round 2, on 09e5b25, none either).

Online reviews on b54720f: OpenAI's P2 (a zero limit does not stop a pipe handler such as Apport) and pullfrog's README thread (the same point at the user guide) are fixed by the limit of 1, which the kernel enforces whatever the handler; the README's claim now holds as written. On 09e5b25, OpenAI reported nothing new, and pullfrog noted that a socket core_pattern (@, Linux 6.16+) bypasses the kernel's skip at 1; fixed in the host.rs comment, since systemd-coredump, the only socket handler, reads the crashed process's soft limit and declines below a page (coredump-context.c, coredump-submit.c).

Follow-ups

  • Unrelated flake seen while testing: the helper sometimes exits with SIGTRAP at startup (GPU process isn't usable. Goodbye. after "Failed to create shared context for virtualization"). That is quest/m0/cef-startup.md, claimed by moq-dev/studio#49.

🤖 Generated with Claude Code

kixelated and others added 3 commits November 4, 2026 23:27
A renderer killed by chrome://crash on its first navigation stayed Starting
past the test's budget on NixOS. CEF does report it, but only once the
process is gone, and systemd-coredump took 20 to 70 seconds to dump the
crashed renderer first. The helper now sets its core dump limit to zero
before CEF starts, which CEF's subprocesses inherit, so the crash reaches
the studio within milliseconds.

The crash test splits in two: a renderer that crashes during its first
navigation, and one aborted after the page has drawn.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title A browser renderer that dies on its first page fix(scene): a browser page that crashes while loading fails at once Nov 5, 2026
@kixelated
kixelated marked this pull request as ready for review November 5, 2026 08:03
@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 49 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4c7fc3e7-6283-488d-b77f-c0ec65546e76
Commits

Reviewing files that changed from the base of the PR and between c797c52 and a550a59.

Files selected for processing: 5

  • README.md
  • quest/m0/README.md
  • quest/m0/browser-first-crash.md
  • rs/moq-scene/src/browser/host.rs
  • rs/moq-studio/tests/browser.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: b54720f

  • [P2] Clarify or cover piped core handlers that ignore a zero limit (rs/moq-scene/src/browser/host.rs:118–125). On Linux, RLIMIT_CORE=0 is ignored for piped core handlers except for the special value 1. Apport can still capture and compress a report core, so renderer termination can remain delayed on such hosts despite the immediate-failure goal. If that guarantee includes Apport/custom-handler systems, use suppression enforced independently of the handler; otherwise narrow the stated guarantee and document the limitation.

Setting the limit in the separate helper before CEF initialization is otherwise a focused approach, and existing failure/reload paths appear coherent.

Verification: GitHub-only static review against Linux and Apport sources. I did not run real-CEF tests or reproduce on an Apport host.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ One documentation suggestion inline: the README explains the immediate failure with a mechanism the soft limit can't guarantee on every core handler. The OpenAI review already raised this at the code site; this anchors it at the user-facing promise.

Reviewed changes

Reviewed the five-file diff at b54720f statically — the review shell has no CEF, Vulkan or moq submodule, so just render-sw could not run here and the new tests were read rather than executed.

  • Renderer crash during load now fails promptly — browse() lowers the helper's soft RLIMIT_CORE to 0 before cef::initialize(), so the zygote, renderers, GPU and utility processes inherit it and a crashed renderer is reaped, and reported, without waiting on a multi-GB dump. Only the soft limit is touched, and the studio's own process keeps its limit.
  • Crash coverage split in two — a_crash_while_loading_drops_only_its_source keeps the earlier chrome://crash case, and the new a_crash_after_drawing_leaves_the_canvas aborts a drawing page's renderer, then checks the fail reason, the canvas and the reload.
  • /proc inspection factored out — helper() now sits on a shared processes() snapshot, and renderers() walks the helper's descendants for the Compositor thread instead of duplicating the scan.
  • Docs and quest updated — the README states either kind of browser crash shows at once, and the completed quest/m0/browser-first-crash.md is gone from both the tree and the milestone index.

The root cause and the fix itself check out: setting the limit in the helper before CEF starts is the right layer, and systemd-coredump declining the dump at a zero soft limit is what turns the 20–70 s stall into the measured ~73 ms.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using big-pickle (free) | 𝕏

Comment thread README.md Resolved
kixelated and others added 2 commits November 5, 2026 06:43
The kernel ignores RLIMIT_CORE for a pipe core_pattern, so a handler that
reads the core regardless (Apport, a custom one) could still hold a crashed
renderer for a minute. At exactly 1 the kernel skips the handler itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Contributor Author

On the OpenAI P2 (host.rs:118-125, a zero limit does not stop a pipe handler such as Apport): confirmed against fs/coredump.c and fixed in 09e5b25. The soft limit is now 1, which makes the kernel skip any pipe handler; 0 relied on the handler honouring %c. Details and the NixOS check are in the PR body.

(Written by Claude Opus 5.5)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit 09e5b25. No new actionable findings in the changes since b54720f, accounting for merged base updates.

The earlier piped-core-handler concern is addressed when the inherited hard limit permits 1: Linux skips the pipe handler at that value. The remaining hard-limit-0 case is documented and suppression there still depends on the handler. The direction is reasonable given that the hard limit is preserved.

Verification: GitHub-only static review of the updated code, surrounding failure paths, tests, and Linux coredump implementation. I did not run builds or real-CEF tests.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ One scoping note inline: the kernel-enforced skip covers file and pipe core_patterns, but not the newer @ socket mode. The change itself is correct, and the pipe case is now handler-independent.

Reviewed changes

Re-reviewed the delta since the prior pullfrog review at b54720f — one commit (09e5b25), read statically; this shell has no CEF or Vulkan, so the browser tests were not executed.

  • Soft core limit raised from 0 to 1 — browse() now sets the helper's soft RLIMIT_CORE to min(hard, 1) instead of 0, so the kernel itself aborts a piped dump at exactly 1 rather than depending on systemd-coredump declining a zero limit.
  • Comment records the new mechanism and its remaining gap — the host.rs comment explains why 1 rather than 0, and that a hard limit of 0 forbids 1 so a pipe handler can still dump.

The kernel behaviour checks out against fs/coredump.c: coredump_pipe() returns early at cprm->limit == 1, and a file dump requires at least binfmt->min_coredump. The min(hard, 1) expression is safe — it preserves the hard limit and cannot exceed it, and it stays 0 only when the hard limit already forbids 1, where setrlimit would otherwise fail.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using big-pickle (free) | 𝕏

Comment thread rs/moq-scene/src/browser/host.rs Resolved
@kixelated
kixelated merged commit 98d52d2 into main Nov 5, 2026
4 checks passed
@kixelated
kixelated deleted the quest/m0/browser-first-crash branch November 5, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant