fix(obsidian): resolve headless renderer crash from Chrome 140 Fontations bug - #1460
Conversation
…ions bug Chrome 140's Fontations font indexer crashes with NOTREACHED in remote_font_face_source.cc:357 when zero system fonts are installed. This killed the Electron renderer, preventing obsidian-git from running. - Add --disable-features=FontationsFontIndexer to bypass the buggy Rust font indexer and fall back to FreeType - Add dejavu_fonts and fontconfig packages so the renderer has fonts - Add ExecStartPre singleton lock cleanup to prevent stale locks from blocking service restarts - Use larger xvfb screen (1280x1024x24) for better rendering
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe Obsidian headless service setup adds font runtime dependencies and Xvfb screen configuration. A systemd pre-start cleanup step removes singleton coordination artifacts from the config directory before service startup. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Mesa DescriptionTL;DRFixed Obsidian headless service crash caused by a Chrome 140 Fontations font indexer bug. What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Code Review
This pull request updates the headless Obsidian service by adding font dependencies (dejavu_fonts, fontconfig), configuring Xvfb screen settings, and disabling the FontationsFontIndexer feature. It also introduces an ExecStartPre script to clean up Obsidian singleton lock files. Feedback was provided to ensure the configuration directory exists before the cleanup script runs to prevent potential execution failures.
| ExecStartPre = "${pkgs.writeShellScript "obsidian-pre" '' | ||
| rm -f ${config.xdg.configHome}/obsidian/SingletonLock | ||
| rm -f ${config.xdg.configHome}/obsidian/SingletonSocket | ||
| rm -f ${config.xdg.configHome}/obsidian/SingletonCookie | ||
| ''}"; |
There was a problem hiding this comment.
The ExecStartPre script will fail if the directory ${config.xdg.configHome}/obsidian does not exist (e.g., on a fresh installation or after a manual cleanup). This is because pkgs.writeShellScript enables set -e by default, and rm -f returns a non-zero exit code if a component of the path (the parent directory) is missing. Adding mkdir -p ensures the directory exists, and quoting the paths handles potential spaces in the configuration directory path.
ExecStartPre = "${pkgs.writeShellScript "obsidian-pre" ''
mkdir -p "${config.xdg.configHome}/obsidian"
rm -f "${config.xdg.configHome}/obsidian/SingletonLock"
rm -f "${config.xdg.configHome}/obsidian/SingletonSocket"
rm -f "${config.xdg.configHome}/obsidian/SingletonCookie"
''}";
There was a problem hiding this comment.
🧹 Nitpick comments (1)
home-manager/services/obsidian/obsidian-headless.sh (1)
2-2: Add regression checks for the new launch arguments.The current spec (
spec/obsidian_headless_spec.sh:1-46) doesn’t assert-s "-screen 0 1280x1024x24"or--disable-features=FontationsFontIndexer, so these can regress silently.Proposed spec additions
Describe 'placeholder references' @@ It 'uses xvfb-run with auto-servernum flag' When run bash -c "grep '\\-a' '$SCRIPT'" The output should include '-a' End + +It 'sets explicit Xvfb screen geometry' +When run bash -c "grep -- '-s \"-screen 0 1280x1024x24\"' '$SCRIPT'" +The output should include '-screen 0 1280x1024x24' +End + +It 'disables Fontations font indexer' +When run bash -c "grep -- '--disable-features=FontationsFontIndexer' '$SCRIPT'" +The output should include '--disable-features=FontationsFontIndexer' +End End🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@home-manager/services/obsidian/obsidian-headless.sh` at line 2, The test suite is missing assertions for the new launch args so regressions can slip by; update spec/obsidian_headless_spec.sh to assert that the generated command line from obsidian-headless.sh (the exec line using xvfb-run in obsidian-headless.sh) includes the -s "-screen 0 1280x1024x24" option and the --disable-features=FontationsFontIndexer flag (you can also assert --disable-gpu if desired) by adding explicit string/regex checks against the produced command or script content rather than only checking other flags.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@home-manager/services/obsidian/obsidian-headless.sh`:
- Line 2: The test suite is missing assertions for the new launch args so
regressions can slip by; update spec/obsidian_headless_spec.sh to assert that
the generated command line from obsidian-headless.sh (the exec line using
xvfb-run in obsidian-headless.sh) includes the -s "-screen 0 1280x1024x24"
option and the --disable-features=FontationsFontIndexer flag (you can also
assert --disable-gpu if desired) by adding explicit string/regex checks against
the produced command or script content rather than only checking other flags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d21d93e-7e49-468e-9fa8-2a4ab4412b39
📒 Files selected for processing (2)
home-manager/services/obsidian/default.nixhome-manager/services/obsidian/obsidian-headless.sh
Summary
NOTREACHEDinremote_font_face_source.cc:357)--disable-features=FontationsFontIndexerto fall back to FreeTypedejavu_fontsandfontconfigso the renderer has system fontsExecStartPresingleton lock cleanup to prevent stale locks on restartContext
The Electron renderer was crashing because the server had zero fonts installed (
fc-listreturned 0). Chrome 140's new Rust-based Fontations font indexer callsGetLastResortFallbackFont()which returns nullptr with no fonts, triggering a fatalNOTREACHED. This prevented the obsidian-git plugin from ever executing.Upstream: chromium#442747781, chromium#436609543
Test plan
systemctl --user status obsidianshows active (running)Summary by cubic
Fixes the headless Obsidian crash on Chrome 140 by disabling the Fontations font indexer and installing system fonts. The service now starts reliably and the obsidian-git plugin can run.
--disable-features=FontationsFontIndexerto fall back to FreeType.dejavu_fontsandfontconfigso the renderer has fonts.Written for commit c17408b. Summary will update on new commits.