Skip to content

fix(load): stop an overtaken load from stranding a document on its slice (#547) - #552

Merged
PathGao merged 1 commit into
masterfrom
fix/large-file-load-race-547
Aug 8, 2026
Merged

fix(load): stop an overtaken load from stranding a document on its slice (#547)#552
PathGao merged 1 commit into
masterfrom
fix/large-file-load-race-547

Conversation

@PathGao

@PathGao PathGao commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

What this is

One guard, applied to every write a load makes instead of only to the last one.
Closes #547, reported by @PathGao.

A document over 50KB is read twice: open_markdown_preview returns the first
50KB so something renders at once, and a background full read replaces it. #247
gave every load a revision and made the second stage refuse to apply a stale
result. The first stage was left unguarded, across two awaits.

Mechanism

The startup path delivers one filename on two channels that nothing dedupes:

where what it does
push lib.rs:3063 setup() emits file-path with argv[1]
pull window_runtime.rs:575 send_markdown_path re-reads std::env::args()
both lib.rs:3155-3160 RunEvent::Opened pushes onto startup_files and emits file-path

The frontend listens for the event (MarkdownViewer.svelte:3087, not awaited) and
separately drains the stash (:3366). So two loads run on one tab, and which
preview read returns first is a coin flip:

t=0    load A  rev=1  -> open_markdown_preview (slow)
t=0+   load B  rev=2  -> open_markdown_preview (fast)
t=110  B stage 2 lands -> whole file, isTruncated false   <- correct state
t=120  A stage 1 lands -> 50KB slice, isTruncated true    <- overwrites it
t=220  A stage 2       -> rev 2 != 1, correctly refused

A destroys the good state and then declines to repair the damage it caused.
Nothing retries. The tab keeps isTruncated, and every save from then on is
refused — surfaced through the auto-save timer, so a load failure is announced as
a save failure.

Which of canApplyFullLoad's five conditions fails is only ever the revision:
path, isDirty, isEditing and isSplit are all unchanged at bail time.

Not dev-only, and not a command line quirk. None of the three channels is
gated on cfg(dev)RunEvent::Opened is gated on target_os = "macos" only,
and it is the Finder path: double-click, Open With, drag onto the icon.
tauri.conf.json registers fileAssociations, so double-clicking a .md file is
this path. A dev build makes the race more observable — slower renders, a debug
Rust build, Vite serving thousands of modules all widen the window — not more
real.

The guard has to sit ahead of setTabDecodedLossy/setTabEncoding and not merely
ahead of the buffer. #544 made the encoding verdict part of the same write, a 50KB
byte cut can split a multi-byte character (utf8_truncation_boundary in
lib.rs:476 is UTF-8 only, and samples/encoding-gbk-large.md is 103,723 bytes
of GBK), and tab.encoding is what the save writes with. A stale prefix's verdict
is the wrong one.

The non-markdown branch takes the same guard. It reads the whole file so it cannot
strand a slice, but a stale one still overwrites the winner's buffer and encoding
and flips the tab into the editor.

Scope

The blank pane is not fixed here. A tab can also carry isTruncated with an
empty buffer, from markTabContentUnavailable() when session restore defers or
fails a read (windowSession.svelte.ts:255, :283). That is the other half of
the report, it is a different cause that happens to share one flag, and a guard on
this race does not touch it. Worth its own issue.

The refusal message is left as it is. It is an untranslated English string
where toast.partialDocument already exists, translated, and is what all five
editing entry points show for this flag. Swapping it was tried and dropped: that
sentence says "cannot edit yet", and by the time a save is refused the reader is
already editing — so it is accurate for the entry points and wrong here. Doing it
properly means a new key in 26 languages, in the shape of lossySaveBlocked
(what happened, why the write is refused, the way out), for a state this change is
meant to make unreachable. Separately: saveContentAs refuses too, so unsaved
edits in this state have no exit at all, where the lossy refusal deliberately
leaves Save As open as one. Both belong with the blank-pane issue.

Noticed and not fixed: the 50KB threshold has no recorded measurement. It
arrives in 31adc64 with a one-line message and no numbers. On this machine,
reading samples/stress-test.md whole is 0.032ms against 0.022ms for its first
50KB — and the two-stage path then reads it whole anyway and renders twice. The
mechanism earns its keep on multi-MB documents, roughly two orders of magnitude
above where it currently engages; the render cost is the part still unmeasured, and
it has to be measured in the app. Mainstream editors do not hand out a partial
writable buffer at all: VS Code declines to display a file past its limit and
offers a Configure Limit button, and Emacs prompts at large-file-warning-threshold
and offers find-file-literally, which turns features off rather than showing less
of the document. Both degrade capability, never fidelity. Markpad's preview
slice degrades fidelity, which is safe only under the invariant that a partial
buffer is never writable — the invariant this race broke. Its own issue.

Tests

scripts/largeFileLoadRevision.test.ts was 18 lines of source regex, which is why
nothing caught this: it passes with the bug present, and it still does. It now also
drives the real TabManager and the real document session through every ordering.

Revert the three guards and three of the four new tests go red — the stranded
slice, the refused save, and the ordering sweep. The regex test stays green, which
is the point of adding behavioural ones next to it.

Verification

npm audit       0 vulnerabilities
npm run check   674 files, 0 errors, 0 warnings
npm test        940 pass, 0 fail
cargo test      145 passed, 0 failed   (no Rust file changes on this branch)

Not verified: reproduced in a packaged build. The race needs the first load's
preview read to land more than ~100ms after the second load's full read has been
applied through its idle callback, and both reads are of the same file issued
milliseconds apart — once it is in the page cache they are both fast and the gap
closes. Repeated launches by hand did not produce it. The evidence that the state
is reachable and that the guard closes it is the test harness, which reproduces the
exact end state (isTruncated, 50,000-byte buffer, saveContent false) and the
exact interleaving deterministically.

No file was ever corrupted by this: the refusal held every time. What broke was the
tab becoming permanently unsaveable while looking normal, and saying so as if the
save were the problem.

🤖 Generated with Claude Code

…ice (#547)

A document over 50KB is read twice: `open_markdown_preview` returns the first
50KB so something renders at once, and a background full read replaces it. #247
gave every load a revision and made the second stage refuse to apply a stale
result. The first stage was left unguarded, across two awaits.

The startup path delivers a file on two channels that nothing dedupes. Rust's
setup emits `file-path` with argv (`lib.rs:3063`) and `send_markdown_path`
re-reads `std::env::args()` (`window_runtime.rs:575`); on macOS
`RunEvent::Opened` does both in the same handler — pushes the path onto
`startup_files` AND emits `file-path` (`lib.rs:3155-3160`). The frontend listens
for the event (`MarkdownViewer.svelte:3087`, not awaited) and separately drains
the stash (`:3366`). So two loads run on one tab, and which preview read returns
first is a coin flip:

    t=0    load A  rev=1  -> open_markdown_preview (slow)
    t=0+   load B  rev=2  -> open_markdown_preview (fast)
    t=110  B stage 2 lands -> whole file, isTruncated false   <- correct
    t=120  A stage 1 lands -> 50KB slice, isTruncated true    <- overwrites it
    t=220  A stage 2       -> rev 2 != 1, correctly refused

A destroys the good state and then declines to repair the damage it caused. The
tab keeps `isTruncated`, nothing retries, and every save from then on is refused
with 'Refusing to save a partially loaded document' — reported through the
auto-save timer, so a load failure surfaces as a save failure.

Which of `canApplyFullLoad`'s five conditions fails is only ever the revision:
path, isDirty, isEditing and isSplit are all unchanged at bail time.

The guard is the one the second stage already applies, moved to cover every write
a load makes. It has to sit ahead of `setTabDecodedLossy`/`setTabEncoding` and not
merely ahead of the buffer: #544 made the encoding verdict part of the same write,
a 50KB byte cut can split a multi-byte character (`utf8_truncation_boundary` in
`lib.rs:476` is UTF-8 only, and `samples/encoding-gbk-large.md` is 103,723 bytes
of GBK), and `tab.encoding` is what the save writes with. A stale prefix's verdict
is the wrong one.

The non-markdown branch takes the same guard. It reads the whole file so it cannot
strand a slice, but a stale one still overwrites the winner's buffer and encoding
and flips the tab into the editor.

Not fixed here: a tab can also carry `isTruncated` with an empty buffer, from
`markTabContentUnavailable()` when session restore defers or fails a read
(`windowSession.svelte.ts:255`, `:283`). That is the blank-pane half of the
report, it is a different cause that happens to share the flag, and a guard on
this race does not touch it. Also left alone: the refusal message is an
untranslated English string where `toast.partialDocument` already exists,
translated, and is what every editing entry point shows for this state.

`scripts/largeFileLoadRevision.test.ts` was 18 lines of source regex, which is
why nothing caught this — it passes with the bug present. It now drives the real
TabManager and the real session through both orderings. Revert the guards and
three of the four new tests go red; that regex test stays green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@PathGao
PathGao merged commit 4e64ae5 into master Aug 8, 2026
4 checks passed
@PathGao
PathGao deleted the fix/large-file-load-race-547 branch August 8, 2026 13:08
@PathGao

PathGao commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

根因分析正确。三个入口(setup() emit、send_markdown_pathRunEvent::Opened)在同一个 tab 上产生两次 load,第一阶段缺少 revision 守卫是唯一的缺口。

修复逻辑:在每次写入 buffer 之前检查 revision,而不只是第二阶段结束时检查。改动最小——只加了守卫,没有动任何状态机。测试覆盖了四种顺序,能抓到 overturned load。

一个未解决的问题(但属于独立 issue):50KB 阈值本身没有测量依据。121KB 文件全读 0.032ms,50KB 切片 0.022ms——差异在微秒级。这个机制在多 MB 文件上才有意义。但这是阈值的问题,不是竞态的问题。

PathGao added a commit that referenced this pull request Aug 8, 2026
PR #552 introduced largeFileLoadRevision.test.ts.  PR #553 added the
onPartialCopySaved callback to DocumentSessionOptions but did not update
this test stub.  svelte-check fails on master because the required
property is missing.
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