Repository navigation
fix(scene): a browser page that crashes while loading fails at once - #50
Conversation
Quest-Claim: e8c9d494441c8465
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>
|
Warning Review limit reachedYou'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. View limit details
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. |
kixelated
left a comment
There was a problem hiding this comment.
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=0is 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.
There was a problem hiding this comment.
ℹ️ 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 softRLIMIT_COREto 0 beforecef::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_sourcekeeps the earlierchrome://crashcase, and the newa_crash_after_drawing_leaves_the_canvasaborts a drawing page's renderer, then checks the fail reason, the canvas and the reload. /procinspection factored out —helper()now sits on a sharedprocesses()snapshot, andrenderers()walks the helper's descendants for theCompositorthread 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.mdis 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.
big-pickle (free) | 𝕏
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>
|
On the OpenAI P2 ( (Written by Claude Opus 5.5) |
kixelated
left a comment
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
ℹ️ 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 softRLIMIT_COREtomin(hard, 1)instead of 0, so the kernel itself aborts a piped dump at exactly 1 rather than depending onsystemd-coredumpdeclining a zero limit. - Comment records the new mechanism and its remaining gap — the
host.rscomment 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.
big-pickle (free) | 𝕏
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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(nowa_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_patternpipes the core to systemd-coredump, and a dying process stays alive until its dump finishes. For thechrome://crashrenderer that took 21 seconds in one run and 67 seconds in another (5.9G memory peak, 527MB written;journalctlshowsprocessing...at the crash anddumped corethat 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 softRLIMIT_COREto 1. CEF's zygote, renderers, GPU and utility processes inherit it. For a pipecore_patternthe kernel ignores the limit except at exactly 1, where it skips the handler itself (fs/coredump.ccoredump_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://crashas 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 theCompositorthread 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
%cit 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.kill -SEGVleft the renderer running, apparently because V8's WebAssembly trap handler takes a SIGSEGV no fault raised. A page cannot crash itself either: its ownlocation = 'chrome://crash'is silently blocked.Rejected
PR_SET_DUMPABLE 0: execve resets it, so each subprocess would need it again, and it also changes/procownership and ptrace access.Validation
On the NixOS host (
ssh nixos), in a scratch checkout, withjust render-sw's browser setup (lavapipe,xvfb-run):main:a_crash_drops_only_its_sourcefails with "renderer never failed" after 20s.b54720f): all 7 browser tests pass, three runs in a row (9 to 16s per run).09e5b25, merged withmain): 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 runa_page_draws_and_playsfailed once with "the page's tone never reached the bus"; that test crashes nothing, and it passed in the three runs after.host.rschange reverted and the new tests kept,a_crash_while_loading_drops_only_its_sourcefails as onmain, anda_crash_after_drawing_leaves_the_canvaspasses. 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.cargo clippy --workspace --all-targetswith and without--features moq-scene/capture,moq-studio/capture,moq-studio/openh264,moq-studio/cefand-D warnings: clean.cargo fmt --check: clean.-D warnings, andcargo fmt --check, on09e5b25: clean.just cion macOS, on head09e5b25(after mergingmainwith 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 heada06dd0aa3469f0517b4637525d69f3cf6a3d91c3reported no findings (round 2, on09e5b25, 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. On09e5b25, OpenAI reported nothing new, and pullfrog noted that a socketcore_pattern(@, Linux 6.16+) bypasses the kernel's skip at 1; fixed in thehost.rscomment, 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
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