dd9493bdbd
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| f54ca2f81a |
IME: map commits against the whole buffer; drop the renderer-side model
The renderer kept a mirror of the pushed IME snippet (the 'IME model') to translate commit positions, but it transiently desynced from the buffer on fling/tap sequences (observed as a few-byte mapping drift on both the x86_64 emulator and the ARM phone), corrupting text. The model string also sat on the main goroutine next to the JNI render path, where the app observed states that were impossible for Go memory (string contents changing between reads microseconds apart), pointing at corruption in the native bridge layer. Restructure along the lines of the Android InputConnection contract and Gio's own reference editor (widget/editor.go): - Commits carry absolute file runes (the pushed snippet's coordinate space) straight to the logic goroutine, which maps them to bytes against the WHOLE buffer (runeToByteWhole, an 8 KiB-step scan). Scrolling moves the window, not the buffer, so the mapping is exact mid-fling by construction — no mirror to desync. - Drift guard in HandleIMECommit: a small commit (range <= 2 runes) is always anchored at the caret the IME was last told about; if the IME reports it ending elsewhere, its snippet text is stale (a dropped restartInput, as Gboard does during flings) and its position is in the stale text's coordinates — snap the commit to the cursor, the only position it cannot drift from. - FlushIME simplifies to: push the snippet when the frame's (context+window) text differs from the last push (gioui dedupes against its own cache), force the selection re-push in the same frame. After a commit the frame text equals what the IME already holds locally, so the restart is naturally suppressed; a fling re-anchors the IME once per text change. - Remove the renderer model (adoptFrame/ModelTranslate/ ApplyIMEEdit/ApplyIMEKey/IMECaret), the IME freeze/settle machinery (IMEFrozen, markIMEScrollActive, imeSettleChan), and the window-relative imeRuneToByte. Also fixed along the way (both found while chasing the corruption): - real.ReadFileAt: loop over short reads. A single ReadAt on Android FUSE can return a short read, silently truncating a chunk and shifting every byte offset after it. - logic: a late lazy-chunk result no longer clobbers a buffer that SetContent has already fully loaded. - e2e: large-file IME test (1.6 MB file, fling + commit). - app icon (scripts/make_icon.py + cmd/pad/appicon.png) so gogio builds the mipmap/adaptive icon set. Verified: go vet + staticcheck, go test -race (all packages), and the emulator scenario loop (open moby excerpt, fling to mid-file, tap, type 'a', byte-compare the saved file) 75/75 clean. |
|||
| 6c6a0c1a27 |
Fix write-concurrency race: per-file write protocol + unique temps
The review-identified race: saves are async (owner snapshots content,
worker pool writes), nothing serialized per file, and every write of a
file used the SAME deterministic temp ('.<name>.tmp'). Two overlapping
writes (autosave x autosave, retry x autosave, or the synchronous
FlushAll on GoToBrowser/Shutdown x a worker write) interleaved on the
shared temp and could rename a byte-mixture into place; even without
interleaving, last-rename-wins could promote a STALE snapshot.
Owner-side protocol (logic.go, requestSave + result handler):
- at most one write in flight per file (writeInFlight maps filename ->
the file version whose content the in-flight write carries);
- a save requested while one is in flight is deferred (savePending) and
re-issued by the write's result handler with a FRESH snapshot, so
'last rename wins' coincides with 'newest snapshot wins';
- on success the SNAPSHOT version (not the current one) is recorded as
written, so an edit that arrived during the write leaves the file
dirty and triggers the re-issue;
- FlushAll (GoToBrowser, Shutdown) defers via the same protocol instead
of writing concurrently on the shared temp;
- shutdown drain: on done the owner waits (bounded 5 s) for in-flight
writes and armed retries to settle before exiting, so the post-exit
synchronous FlushAll and workerPool.Stop cannot race a straggling
worker write;
- retry timer now sends a non-blocking token (no timer-goroutine stall
on a full channel); emitFrame no longer blocks on a slow/gone main
(frames are snapshots; the next emission wins) - also required so the
drain can never deadlock on frame delivery.
Mechanism (real/filesystem.go):
- WriteFileAtomic uses a unique per-call temp ('.<name>.tmp.<pid>.<seq>'),
making same-file staging-file interleaving structurally impossible even
if the serialization regressed (defense in depth);
- each successful write best-effort removes stale temps of the same file
(crash leftovers, plus the legacy deterministic name for upgraded
installs); a failed write removes its own temp.
Tests:
- write_serialization_test.go (e2e): a counting FS wrapper proves the
peak concurrent same-file saves is 1 across two deliberately
overlapping autosaves (2 s saves; the second edit lands inside the
first save's window and its token is deferred, then re-issued with the
newer content), and that a Flush during an in-flight save adds no
concurrent writer and the newest snapshot still wins. Mutation-verified:
disabling the deferral fails it with peak = 2. (The pool's WriteFileTask
calls FS.WriteFile, not WriteFileAtomic - the real FS is atomic only
because WriteFile delegates to WriteFileAtomic; the wrapper mirrors that
delegation or the overlap window does not exist.)
- filesystem_test.go: stale-temp test updated to the new pattern, also
covering the legacy name and asserting a different file's temp is
untouched.
- real_file_fuzz_test.go: stray-temp check matches both patterns.
Docs: architecture.md 6.5 rewritten (protocol invariants), spec.md
autosave line, development_plan.md v11 + Phase 12.
On-device smoke: open, type, autosave lands exact content on disk, no
temp files left, clean relaunch. Full suite green under -race.
Residuals (documented): no fsync before rename (power-loss window only);
external-change detection absent; a drain-deadline exit with a straggling
write can only lose freshness (unique temps keep every rename a complete
snapshot).
|
|||
| 5205906de4 |
Data-corruption test suite: differential fuzz + atomicity contract
Adds the corruption-proofing layer for the edit/persist path: - chunked_buffer_fuzz_test.go: differential fuzz of ChunkedBuffer Insert/Delete against a plain []byte shadow model (arbitrary byte positions/text, chunk sizes 1..64KB, 500-2000 ops each) verifying FileLen, FullContent, Content probes and the chunk-size invariant after every op; rune-aligned variant adds UTF-8 validity and an independent RuneIndexToByte oracle. - line_index_fuzz_test.go: differential fuzz of the incremental LineIndex updates against a full-recomputation oracle (mixed, no-newline, single-line, CRLF, trailing-newline shapes), plus a trailing-empty-line structural invariant test. - state_api_fuzz_test.go: differential fuzz of the production edit entry points (HandleInsert/Backspace/Delete/ReplaceRange, incl. selection variants) checking content, UTF-8 validity, chunk invariant, line index and exact cursor after every op. - real_file_fuzz_test.go (e2e): random edit sequences (incl. window-relative IME replaces) against the REAL filesystem with per-batch IME-window-consistency checks, forced-flush disk byte-comparison, then a second logic instance (restart simulation) must reload byte-identical content with a fresh line index; asserts no stray temp files. - filesystem_test.go (real): atomicity contract - exact round trip at edge sizes, concurrent reader never sees a torn file across 150 alternating 2MB writes, failed write (read-only dir) leaves the original byte-identical, stale temp file is consumed. Fixes a real invariant violation the fuzzing exposed: Insert halved an oversized spliced result once, so a large paste into a non-empty buffer left chunks up to ~P/2 (8x target at 1MB/64KB), breaking the documented 'no chunk > 2x target' invariant. Insert now re-chunks the oversized result into pieces of at most chunkSize, making the invariant hold after every edit. Mutation-tested: dropping one byte in Insert and one entry in UpdateLineIndexAfterInsert are both caught by the fuzz suite. |
