Skip to content

fix(sync): re-poll host path after own-write suppression clears - #271

Merged
tstapler merged 5 commits into
mainfrom
fix/host-sync-suppressed-write-repoll
Aug 14, 2026
Merged

fix(sync): re-poll host path after own-write suppression clears#271
tstapler merged 5 commits into
mainfrom
fix/host-sync-suppressed-write-repoll

Conversation

@tstapler

Copy link
Copy Markdown
Owner

Summary

  • Fixes a silent data-loss bug on the wasmJs/web sync path: pollHostDirectoryOnce's own-write suppression guard skipped a path's poll entirely while an app-initiated write-through was in flight for that path (hostWriteInFlight) — never refreshing hostModTimes/hostFileSizes/cache for it. A genuine external edit landing on the same path during that window was dropped with no error/log, since FileRegistry.detectChanges never saw an advanced mtime and GraphFileWatcher never fired.
  • Tracks suppressed paths in a new hostWriteSuppressedDuringFlush set and re-runs pollHostDirectoryOnce once the owning flush clears hostWriteInFlight, bounding the loss window to one extra poll instead of indefinite silent drops.

Root cause confirmed by direct code read of HostDirectorySync.kt's visit()/scheduleHostWriteThrough (not just inferred from symptoms). Caveat: this requires the external edit to land during the exact window of an app-initiated write-through to the same path — plausible for actively-autosaved journal/page files (matches the reported journals/2026_08_13.md repro pattern) but not proven to be the exact original incident's timing.

Test plan

  • JAVA_HOME=.../openjdk-21 ./gradlew :kmp:compileKotlinWasmJs -PenableJs=true — BUILD SUCCESSFUL
  • Manual repro on deployed web build: edit a journal file on disk during active autosave and confirm the change now appears in-app

🤖 Generated with Claude Code

tstapler and others added 3 commits August 13, 2026 20:02
pollHostDirectoryOnce's own-write suppression (added to stop the poller
from misclassifying an in-flight write-through as an external change)
skipped a path's poll entirely while hostWriteInFlight held it -
including the mtime/size/cache refresh. A genuine external edit landing
on that same path during the window was never picked up: hostModTimes
never advanced, so FileRegistry.detectChanges saw no change and
GraphFileWatcher never notified the UI. No error, no log - the edit was
just gone.

Track suppressed paths in hostWriteSuppressedDuringFlush and re-run
pollHostDirectoryOnce once the owning flush clears hostWriteInFlight,
bounding the loss window to one extra poll instead of indefinite silent
drops.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adversarial review of the previous fix (ad0dc53) found retryStuckHostWrites
cleared hostWriteInFlight in its own finally block without checking
hostWriteSuppressedDuringFlush — reopening the same silent-loss window for
any write whose in-flight ownership passes through the stuck-write retry
path instead of scheduleHostWriteThrough. Factor the repoll-on-clear logic
into repollIfSuppressedDuringFlush and call it from both finally blocks.
Closes the coverage gap flagged in review of ad0dc53/7e55c286: no test
previously drove pollHostDirectoryOnce's suppression guard concurrently
with an in-flight retryStuckHostWrites flush. Adds a hybrid writable +
enumerable OPFS root fixture with a gate to pause a flush mid-attempt,
proving the automatic repollIfSuppressedDuringFlush mechanism (not a
second manual poll) is what brings the cache current.

Verified via wasmJsBrowserTest (headless Chrome): 12/12 passing in
HostDirectorySyncWriteThroughTest, including the new test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@tstapler

Copy link
Copy Markdown
Owner Author

Closed the remaining review gap from the adversarial pass: added a regression test for the suppression/repoll race itself (2666dae).

pollHostDirectoryOnce_should_AutomaticallyRepollAndUpdateCache_When_ExternalPollWasSuppressedDuringInFlightWrite uses a new hybrid writable+enumerable OPFS fixture (makeWritableEnumerableHostRoot) with a gate to pause a flush mid-attempt. It proves the automatic repollIfSuppressedDuringFlush mechanism — not a second manual poll — is what brings the cache current after a write's suppression window closes.

Verified with ./gradlew :kmp:wasmJsBrowserTest -PenableJs=true --tests "*HostDirectorySyncWriteThroughTest*": 12/12 passing (headless Chrome), including the new test. Note CI itself only compiles wasmJs tests today (no headless-Chrome/Karma wiring yet, per .github/workflows/ci.yml:305), so this local run is currently the only executed proof of this suite.

Round-2 convergence: re-read the full ad0dc53b/7e55c286 diff plus this new fixture/test against the current source — no further findings.

@tstapler
tstapler marked this pull request as ready for review August 14, 2026 03:45
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Android Load Benchmark

Instrumented benchmark on an API 30 x86_64 emulator — 500-page synthetic graph.

Comparing 0e167f8f (this PR) vs 43912e31 (baseline)
Device: API 30 x86_64 emulator — 530 pages loaded

Graph Load

Metric This PR Baseline Delta
Phase 1 TTI ↓ 27ms 36ms -9ms (-25%) ✅
Phase 3 index ↓ 3927ms 4090ms -163ms (-4%) ✅

Interactive Write Latency (during Phase 3)

Metric This PR Baseline Delta
Write p95 (baseline) ↓ 10ms 11ms -1ms (-9%) ✅
Write p95 (during phase 3) ↓ 14ms 40ms -26ms (-65%) ✅
Jank factor ↓ 1.4x 3.64x -2.24x (-62%) ✅
Concurrent writes ↑ 19 19 0 (0%)

SAF I/O Overhead (ContentProvider vs direct File read)

Measures Binder IPC cost added by ContentResolver per readFile() call.
Real SAF via ExternalStorageProvider will be higher on device; this is a lower bound.

Metric This PR Baseline Delta
Direct read / file ↓ 0.0ms 0.0ms 0 (0%)
Provider read / file ↓ 0.2ms 0.3ms 0ms (-26%) ✅
IPC overhead ratio ↓ 6x 8x -2x (-25%) ✅
↓ lower is better · ↑ higher is better

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

JVM Load Benchmark (Desktop)

Synthetic in-memory benchmark measuring load performance for the desktop (JVM) app.
Comparing 0e167f8f (this PR) vs 43912e31 (baseline)
Graph config: xlarge — 230 pages

Metric This PR Baseline Delta
Phase 1 TTI ↓ 1ms 1ms 0 (0%)
Phase 2 background ↓ 0ms 0ms 0 (0%)
Phase 3 index ↓ 1ms 0ms +1ms ⚠️
Total ↓ 2ms 1ms +1ms (+100%) ⚠️
Write p95 (baseline) ↓ 22ms 15ms +7ms (+47%) ⚠️
Write p95 (under load) ↓ n/a 0ms
Jank factor ↓ n/a 0x
↓ lower is better
Flamegraphs (this PR) **Allocation** — object allocation pressure (JDBC/SQLite churn)

Alloc flamegraph not available

CPU — method-level hotspots by on-CPU time

CPU flamegraph not available

Top allocation hotspots (this PR) `35.3%` byte[]_[k] `8.6%` java.lang.String_[k] `6.4%` java.util.LinkedHashMap$Entry_[k] `6.2%` int[]_[k] `4.9%` java.lang.Object[]_[k]
Top CPU hotspots (this PR) `97.5%` /usr/lib/x86_64-linux-gnu/libc.so.6 `0.9%` /tmp/sqlite-3.51.3.0-d8a880eb-07c2-4a8c-b21b-8eb36454515c-libsqlitejdbc.so `0.4%` __libc_pwrite `0.2%` fsync `0.1%` SR_handler

tstapler and others added 2 commits August 13, 2026 21:04
Remove the dead writableEnumerableRootGetContent accessor (and its
orphaned JS-side _getContent), and expand the new test's comment to
scope what the final assertion actually proves: the repoll reflects
current on-disk content, not survival of a concurrent external edit
against the local write.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@tstapler
tstapler merged commit 79724e1 into main Aug 14, 2026
16 of 18 checks passed
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