Fix offline SQLite atomic ownership - #72
Conversation
✅ Deploy Preview for rdlabo-ionic-angular-library ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| async runReadSnapshot<T>(read: (reader: OfflineRepositoryReader) => Promise<T>): Promise<T> { | ||
| if (this.#atomicMutationRevision !== null) { | ||
| return this.#queueAtomicOperation(async () => this.#nativeTransaction(await this.#databaseConnection(), () => read(this.#reader()))); | ||
| throw new Error('Use the repository passed to an atomic mutation for snapshot reads.'); |
There was a problem hiding this comment.
🔴 同期処理の実行中に読み取り用スナップショットを要求すると例外で失敗する
アプリ側から要求されたまとめ読み取りが、同期処理の実行中というだけで拒否されるようになり（sqlite-offline-repository.ts:311-312 の throw）、待機ではなく即時エラーになるため、無関係な画面のデータ読み込みがタイミング次第で失敗します。
Impact: バックグラウンド同期と重なったときにだけ、キャッシュ読み込みがランダムに失敗しエラー表示になります。
atomic mutation 中の runReadSnapshot 拒否がオーナー以外にも波及する仕組み
SqliteOfflineRepository.runReadSnapshot は #atomicMutationRevision !== null の間、無条件に例外を投げます。#atomicMutationRevision はプロセス全体で共有される 1 つのフラグで、OfflineReplicaMutationCoordinator.run 経由の enqueue / pull 適用 / ACK 反映などが走っている間ずっと非 null です（projects/kit/offline/src/lib/sqlite-offline-repository.ts:324-351）。
従来はこの場合でも #queueAtomicOperation に積まれて完了できていました。書き込み系（#queueWrite / #transaction）は本 PR で this.#atomicIdle.then(...) による「延期」に統一されている（projects/kit/offline/src/lib/sqlite-offline-repository.ts:871-872, 883-884）のに対し、runReadSnapshot だけ延期ではなく失敗になっているため、OFFLINE_REPOSITORY を直接注入して合成読み取りを行う利用側は、同期の有無に依存した非決定的なエラーを受け取ります。
オーナー自身の誤用検知が目的であれば、オーナー facade 側（#atomicRepository() の runReadSnapshot）でのみ再入を許可し、本体側は書き込みと同様に #atomicIdle 後へ延期する方が整合的です。
Prompt for agents
SqliteOfflineRepository.runReadSnapshot は atomic mutation が実行中の場合に無条件で例外を投げるようになったが、#atomicMutationRevision はリポジトリ全体で共有されるフラグであり、atomic mutation の所有者以外（プロダクトコードが OFFLINE_REPOSITORY を直接注入して合成読み取りを行うケース）にも波及する。従来はこの呼び出しは #queueAtomicOperation にキューイングされて成功していたため、バックグラウンド同期と重なった読み取りだけが非決定的に失敗する回帰になっている。書き込み系の #queueWrite / #transaction は同 PR で this.#atomicIdle.then(...) による延期に統一されているので、runReadSnapshot も同様に atomic 完了後まで延期する方針が整合的。所有者の誤用（atomic operation 内から本体の runReadSnapshot を呼ぶ）を検出したい場合は、延期ではデッドロックになるため、atomic operation のコールバック実行コンテキストを識別できる仕組み（例: facade 経由かどうかのフラグや AsyncLocalStorage 相当の明示的な受け渡し）を用いて所有者のみ例外にすることを検討してほしい。
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (this.#readSnapshotActive) { | ||
| return operation(); | ||
| } |
There was a problem hiding this comment.
🔴 まとめ読み取りの実行中は他の全ての読み取りが未完了の書き込みを待たなくなる
まとめ読み取りの実行中であることを示す状態がリポジトリ全体で 1 つしかないため、無関係な読み取りまで待機処理を飛ばして即実行され（#withCommittedRead 内の分岐 sqlite-offline-repository.ts:829-831）、まだ確定していない古いデータが返ります。
Impact: 別画面の読み取りが、直前に依頼済みの保存内容を反映しない古い値を返すことがあります。
#readSnapshotActive がグローバルなブール値であること
#readSnapshotActive は runReadSnapshot（projects/kit/offline/src/lib/sqlite-offline-repository.ts:314-321）および owner facade の runReadSnapshot（projects/kit/offline/src/lib/sqlite-offline-repository.ts:781-791）でコールバック実行中 true になるインスタンス単位のフラグで、「どの呼び出し元が再入しているか」を区別しません。
そのため snapshot コールバックが await している間に、全く別の場所から呼ばれた getCommands / getReplicaRow などの #withCommittedRead も await this.#writes と #beginReaders() をスキップして即座にクエリを発行します。結果として:
- キュー済みで未コミットの書き込みを待たずに読むため、古い状態を返す（従来は必ず
#writesの完了を待っていた）。 - reader lease（
#activeReaders）に登録されないので、その読み取りが継続している間に後続の書き込みトランザクションが開始され得る。
再入を許可したいのは snapshot コールバック自身（およびその同期的な呼び出し先）だけなので、フラグではなく呼び出し側に明示的に reader / owner facade を渡す、あるいは実行コンテキストを識別できる仕組みで限定する必要があります。
Prompt for agents
SqliteOfflineRepository に追加された #readSnapshotActive は、runReadSnapshot（本体版 sqlite-offline-repository.ts:314-321、owner facade 版 sqlite-offline-repository.ts:781-791）のコールバック実行中に立つインスタンス単位のグローバルなブール値である。#withCommittedRead はこのフラグが立っているだけで await this.#writes と #beginReaders() をスキップするため、snapshot コールバックとは無関係な並行読み取りまで、未コミットの書き込みを待たずに実行され、reader lease にも登録されない。目的は『snapshot コールバック内からの本体リポジトリ再入を許可する』ことなので、再入判定を呼び出し元単位に限定する必要がある。例えば snapshot 実行中に生成した専用の repository/reader ハンドル経由の読み取りだけを直行させる、あるいは再入判定用のコンテキストを明示的に受け渡す設計に変更することを検討してほしい。
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (this.#atomicMutationRevision !== null) { | ||
| return this.#queueAtomicOperation(async () => this.#atomicTransaction(await this.#databaseConnection(), run)); | ||
| return this.#atomicIdle.then(() => this.#queueWrite(run)); |
There was a problem hiding this comment.
🟡 同期処理が連続すると、それ以外の保存処理がいつまでも実行されず待たされ続けることがある
同期処理の実行中に依頼された保存が、完了待ちのキューに登録されないまま先送りされる（#queueWrite 内の this.#atomicIdle.then(...) sqlite-offline-repository.ts:871-872）ため、次の同期処理がすぐ始まると再び先送りされ、実行が遅延し続けます。
Impact: 同期が立て込んでいる間、それ以外の保存がいつまでも完了せず、待っている処理が止まったままになる可能性があります。
先送りされた書き込みが #writes チェーンから外れる仕組み
#queueWrite / #transaction は atomic mutation 実行中に this.#atomicIdle.then(() => this.#queueWrite(run)) として再帰的に延期しますが、この延期中の Promise は this.#writes に登録されません（projects/kit/offline/src/lib/sqlite-offline-repository.ts:871-884）。
atomic mutation の開始処理は await this.#writes しか待たない（projects/kit/offline/src/lib/sqlite-offline-repository.ts:325-326）ため、#atomicIdle の resolve 後に延期分の継続（マイクロタスク）が走るより前に次の atomic mutation が開始されると、#atomicMutationRevision が再び非 null になり、延期分はさらに次の #atomicIdle へ積み直されます。OfflineReplicaMutationCoordinator は enqueue / ACK / pull 適用を連続実行するため、フラッシュ中は atomic mutation が背中合わせに並びやすく、外部書き込みが繰り返し後回しにされ得ます。
延期中の書き込みも #writes 相当のキューに載せ、atomic mutation 開始時にそれを待つようにすれば飢餓を避けられます。
Prompt for agents
#queueWrite と #transaction は atomic mutation 中に this.#atomicIdle.then(() => 再帰) で書き込みを延期するが、この延期中の Promise はどのキュー（#writes）にも登録されない。一方で OFFLINE_REPOSITORY_ATOMIC_MUTATION の開始処理は await this.#writes しか待たないため、#atomicIdle resolve 後のマイクロタスクが動く前に次の atomic mutation が開始されると、延期された書き込みはさらに後ろへ積み直され、coordinator が atomic mutation を連続実行する状況（フラッシュ中の enqueue/ACK/pull 適用）で無期限に遅延し得る。延期中の書き込みも #writes に相当する待機対象として登録し、atomic mutation の開始がそれを待つ（あるいは atomic 終了時に延期分を優先的に flush する）設計に修正することを検討してほしい。
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
What changed
Why
Follow-up to Devin review on #71:
The prior global atomic flag could absorb unrelated writes into the owner queue, and snapshot callbacks could wait on themselves.
Validation
npx ng test kit --watch=false(44 files / 822 tests)npm run prebuild:kitgit diff --check