Write redd-block-data.json atomically to prevent torn reads dropping blocks#43
Open
kasnder wants to merge 2 commits into
Open
Write redd-block-data.json atomically to prevent torn reads dropping blocks#43kasnder wants to merge 2 commits into
kasnder wants to merge 2 commits into
Conversation
…blocks The data file is re-read continuously by the Automation watcher (1 s), the browser-spawned native host (2 s mtime poll), and the enforcer (5 s). Replacing it with a plain truncate-and-write let those readers observe a half-written file: the JSON parse fails, derive_payload() returns an empty blocklist, and blocking momentarily drops — the extension clears its rules and the Automation watcher un-parks tabs from the block page. The native host's mtime poll made this likely, since mtime changes at the start of a write. All writers (save_data, the load_data EULA/location migrations, and the enforcement/grace/blocking-method settings commands) now go through a single helper that writes to a temp file in the same directory, fsyncs, applies the shared permissions, and renames over the destination — atomic on both APFS and NTFS, so readers see either the old or the new complete file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kasnder
force-pushed
the
claude/musing-kirch-e57e60
branch
from
abccd19 to
29085c6
Compare
The test fed path_to_file_url a "ReDD Blocker.app" path but still asserted the pre-rename "ReDD%20Block.app" URL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
redd-block-data.jsonis the single source of truth for "what is blocked right now", and it is re-read continuously by independent readers:Every writer used a plain truncate-and-write (
fs::write), so a reader could observe a half-written file.derive_payload()treats a JSON parse failure as an empty blocklist, which fails open: the extension is pushed an empty list and clears blocking, and the Automation watcher's "no domains → restore parked tabs" logic un-parks tabs from the block page. Users could hit this window on every block edit or settings save.Fix
New
write_data_file_atomic()helper incommands/data.rs: write to a uniquely-named temp file in the same directory,sync_all(), apply the shared-file permissions to the temp file (rename gives the destination the temp file's mode), thenrename()over the destination — atomic on both APFS and NTFS, and Windows readers using std's default share flags are unaffected by the swap. On failure the temp file is cleaned up.All data-file writers now go through it:
save_dataand the threeload_datamigration/EULA-normalization writes (commands/data.rs)set_enforcement_enabled(commands/enforcement_toggle.rs)set_extension_grace_seconds(commands/grace.rs)set_blocking_method(commands/blocking_method_cmd.rs)(The settings commands previously wrote without re-applying shared permissions; going through the helper also fixes that.)
Also includes a one-line fix for
web_automation::tests::file_url_encodes_spaces, which had been failing since the product rename (expected URL still said "ReDD Block.app").Testing
cargo checkclean (only pre-existing warnings)cargo test --lib: 29 passed, 0 failedNot addressed (follow-ups)
derive_payload()still fails open on a genuinely corrupt/missing file; keeping a last-known-good payload would be a further hardening step.preserve_backend_settings/ the settings commands still do unlocked read-modify-write, so a lost-update race between concurrent writers remains possible (much rarer than the torn-read issue fixed here).🤖 Generated with Claude Code