Skip to content

fix(store): synchronize mutable writes by admitted batch - #762

Merged
matt-ramotar merged 6 commits into
mainfrom
matt-ramotar/fix-761-mutable-store-synchronization
Sep 8, 2026
Merged

fix(store): synchronize mutable writes by admitted batch#762
matt-ramotar merged 6 commits into
mainfrom
matt-ramotar/fix-761-mutable-store-synchronization

Conversation

@matt-ramotar

@matt-ramotar matt-ramotar commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #761.

Overlapping writes for one key can currently lose B when A replaces the pending queue, or let an older eager success acknowledge B even though B's update failed. This change keeps stable per-key state and acknowledges only the admitted entries captured before each post. Explicit writes and eager retries share server synchronization, while B can still persist locally during A's network call.

Pending entries use identity rather than request timestamps or equality, and coalesced callers receive the saved server result. Cancellation propagates while retaining admitted work. Success callbacks run after locks and adapter context exit, and callback or bookkeeping failures cannot undo acknowledgment. Inherited recursive adapter operations for the same store and key are rejected before nested effects.

The published version is now 5.1.0-alpha12 in the shared version catalog. Generated JVM and Kotlin Multiplatform Maven metadata, including Store project dependencies, resolve to alpha12.

Validation (synchronization implementation 0274288d770eca167ddcfaf94c461141aea48892; version-only follow-up db1b957c9f8bae2e52304c36b5587e6eb166e9a9):

  • Baseline fde07551688ae7f1e22a8da77f6cbcde1454f384: both deterministic issue tests executed and failed. newerWriteSurvivesAcknowledgementCommit returned an error for B; olderEagerSuccessDoesNotAcknowledgeNewerFailedWrite delivered B's callback once when the expected count was zero. The log, XML, report, fixture, and test source were archived before implementation. Both tests now pass.
  • The barrier cancellation baseline ran 9 tests with the new active-job cancellation assertion failing; the fixed barrier suite passes.
  • Fresh full Store JVM execution: 135 tests, 0 failures, 0 errors, 0 skipped, using ./gradlew :store:jvmTest --rerun-tasks --no-build-cache --console=plain.
  • Fresh MutableStore execution: 50 tests passed, including the deterministic local-responsiveness test and the existing 64-writer, 50-round contention workload, now requiring every response to succeed.
  • ./gradlew :store:apiCheck :store:ktlintCheck :store:spotlessCheck --console=plain passed. The JVM API baseline is unchanged.
  • Fresh iOS Simulator ARM64 execution: 135 tests, 0 failures, 0 errors, 0 skipped, using ./gradlew :store:iosSimulatorArm64Test --rerun-tasks --no-build-cache --console=plain. Native XML includes all 12 acknowledgment tests, 20 lifecycle tests, both contention tests, and all 9 barrier tests; the test executable ran.
  • A focused independent source and evidence review found no blockers or missing required identity/lifecycle test scenarios.
  • DCO sign-offs are present on all six commits and the DCO check passes.
  • Final-commit Linux CI passed at db1b957c9f8bae2e52304c36b5587e6eb166e9a9, including the full build, tests, and coverage upload: CI run.
  • Local version validation passed: ./gradlew :store:generatePomFileForJvmPublication :store:generatePomFileForKotlinMultiplatformPublication --console=plain; both Store publications and their core/cache/multicast dependency coordinates use 5.1.0-alpha12.

The synchronization scope remains one MutableStore instance. Fetch-after-failed-push policy, cross-instance coordination, concurrent clear semantics, and durable outbox behavior are unchanged. The version advances from the already-tagged 5.1.0-alpha11 to 5.1.0-alpha12.


Note

High Risk
Changes core MutableStore concurrency, acknowledgement, and cancellation semantics plus SourceOfTruth write barriers—areas where subtle races or callback reentrancy can cause data loss or deadlocks if wrong.

Overview
Fixes overlapping MutableStore writes for one key that could drop pending work or acknowledge failed updates incorrectly.

RealMutableStore replaces the per-key write queue and ThreadSafety locks with MutableStoreKeyState (separate local vs remote mutexes, identity-based pending entries). Writes persist locally, then synchronize posts the latest snapshot and acknowledges only entries captured in that batch—coalescing callers share one server result. Explicit writes and read-time eager retries share the same path; Bookkeeper markers integrate with snapshot selection.

Adds MutableStoreAdapterContext to reject recursive read/write/clear on the same store/key during adapter/updater/bookkeeper work, documents lifecycle rules on MutableStore, and runs success callbacks after locks release (with isolated failure handling).

SourceOfTruthWithBarrier and RealStore.write now propagate CancellationException and always reopen/release barriers on cancelled writers. ThreadSafety is removed.

Large new test coverage: acknowledgement races, lifecycle/cancellation, concurrency (all writes must succeed), and barrier cancellation.

Reviewed by Cursor Bugbot for commit 0274288. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.84615% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.53%. Comparing base (fde0755) to head (db1b957).

Files with missing lines Patch % Lines
...vefoundation/store/store5/impl/RealMutableStore.kt 92.30% 0 Missing and 7 partials ⚠️
...undation/store/store5/impl/MutableStoreKeyState.kt 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #762      +/-   ##
==========================================
+ Coverage   80.81%   83.53%   +2.72%     
==========================================
  Files          41       42       +1     
  Lines         912      899      -13     
  Branches      174      170       -4     
==========================================
+ Hits          737      751      +14     
+ Misses        107       91      -16     
+ Partials       68       57      -11     
Files with missing lines Coverage Δ
...on/store/store5/impl/MutableStoreAdapterContext.kt 100.00% <100.00%> (ø)
...ilenativefoundation/store/store5/impl/RealStore.kt 90.90% <ø> (+0.75%) ⬆️
...tion/store/store5/impl/SourceOfTruthWithBarrier.kt 98.68% <100.00%> (+1.15%) ⬆️
...undation/store/store5/impl/MutableStoreKeyState.kt 93.75% <93.75%> (ø)
...vefoundation/store/store5/impl/RealMutableStore.kt 93.00% <92.30%> (+12.40%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Matt Ramotar <matt.ramotar@icloud.com>
Signed-off-by: Matt Ramotar <matt.ramotar@icloud.com>
Signed-off-by: Matt Ramotar <matt.ramotar@icloud.com>
Signed-off-by: Matt Ramotar <matt.ramotar@icloud.com>
Signed-off-by: Matt Ramotar <matt.ramotar@icloud.com>
@matt-ramotar
matt-ramotar force-pushed the matt-ramotar/fix-761-mutable-store-synchronization branch from 72961ae to 0274288 Compare September 8, 2026 20:10
@matt-ramotar
matt-ramotar marked this pull request as ready for review September 8, 2026 20:19
Signed-off-by: Matt Ramotar <matt.ramotar@icloud.com>
@matt-ramotar
matt-ramotar merged commit 3f86ddb into main Sep 8, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from 🆕 Triage to ✅ Done in Store Roadmap Sep 8, 2026
@matt-ramotar matt-ramotar mentioned this pull request Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

[BUG] MutableStore can discard pending writes or acknowledge unsynchronized changes

1 participant