Conversation
|
| _profile.update { | ||
| isCurrent = _publicKey.value == pk && profileWriteGeneration.get() == writeGeneration | ||
| if (isCurrent) loadedProfile else it | ||
| } | ||
| if (!isCurrent) { | ||
| Logger.debug("Skipped stale profile load for '${redacted(pk)}'", context = TAG) | ||
| return@onSuccess | ||
| } | ||
| _profile.update { loadedProfile } | ||
| cacheMetadata(loadedProfile) |
There was a problem hiding this comment.
Stale refresh overwrites cached metadata
If a profile refresh overlaps a save or sign-out, it can pass this generation check and then suspend before writing the cached metadata. The newer save or sign-out can finish first, after which the refresh writes the old name, avatar, and owner. The in-memory profile guard does not protect this later write, so a subsequent load can show obsolete details or metadata that sign-out had cleared.
Knowledge Base Used: Pubky identity and profile
There was a problem hiding this comment.
Confirmed and fixed in 979e1fc. loadProfile now passes its current-load check (same key, unchanged write generation) into cacheMetadata, which re-checks it inside the PubkyStore update transform. DataStore runs updates one at a time, so the check and the write happen together relative to saves and sign-out. Sign-out also bumps the generation before it resets the store, so a load write that lands right after the reset is dropped. Covered by three new PubkyRepoTest cases: a held load write after a save, after sign-out, and right after sign-out's store reset.
| ?: return | ||
| runCatching { | ||
| cache.fileSystem.write(editor.data) { write(bytes) } | ||
| if (cacheEpoch.current() == epoch) editor.commit() else editor.abort() |
There was a problem hiding this comment.
The epoch check and commit() are separate operations. If sign-out or an identity switch clears the disk cache after the check passes but before the commit finishes, this fetch can put an avatar back into the cleared cache. That leaves an entry the clear was meant to remove and weakens cache cleanup.
Knowledge Base Used: Pubky identity and profile
There was a problem hiding this comment.
Checked against Coil 3.2.0 and this race can't re-add an entry. The editor is opened before the epoch check, and RealDiskCache.clear() calls DiskLruCache.evictAll(), which marks any entry with an open editor as a zombie; completeEdit() on a zombie entry deletes the dirty files instead of committing them. So a clear that lands between the check and commit() discards this write. I added a comment at the check in 979e1fc so that reliance is explicit; no behaviour change.
There was a problem hiding this comment.
You’re right. Since openEditor() happens before the epoch check, a concurrent RealDiskCache.clear() can evict/zombify that editor; completing it afterward cannot publish the dirty entry, so the write is discarded rather than re-added. The comment in 979e1fc30 makes that Coil 3.2.0 invariant explicit. I’m withdrawing the finding.
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
A Ring row now shows a spinner in its avatar slot until its profile lookup finishes, whatever the outcome, so a row still looking up no longer looks like one whose identity has no profile. The adoption spinner moves into the row's key icon circle, matching iOS.
Twin: synonymdev/bitkit-ios#854
Fixes the lag when loading pubky profiles, which got much worse with the shared Pubky Ring identities from #1329.
Where the lag came from. All Paykit calls go through one lock, and each call kept it for its whole network round trip. That included simple public reads, like fetching a profile name or avatar. So profile lookups queued behind each other, behind sign-in, and behind background Paykit work. The Pubky Ring choice screen made it worse: it looked up every Ring identity one at a time, retried each failure, and hid the whole list behind a spinner until all of them were done. A pubky that was never published takes 2–7 s to fail, and everything else waited for it.
How this fixes it. Public reads skip the lock. They run side by side (up to six at once) and stop when you leave the screen. Anything that touches your session or keys still goes through the lock. The choice screen now shows its rows straight away, with a small spinner on each row until its profile arrives. Adopting a Ring identity reuses the profile its row already found, sign-in no longer waits for the background identity refresh, and avatars are cached on disk.
Description
Out of Scope
app/src/main/java/to/bitkit/repositories/PubkyRepo.kt: app launch still builds the Paykit SDK and restores the session twice; removing that needs Paykit inputrustls_platform_verifier"BKS KeyStore not available" TLS errors on the emulator image; they were there before this PR and can make a lookup failDesign
N/A — no design available. The spinners reuse existing components.
Preview
android-before-after.mp4
Left:
master(dbbf90f). Right: this PR (1f4364f; later commits do not change this flow). Same emulator, same taps, fresh app data, staging network, one take each. After the profile intro,mastershows only "Loading your profile…" for about 11 s. With this PR the rows show at once, each row's spinner turns into its name or avatar, and adopting spins only the tapped row.QA Notes
Journeys
cached-profile-header.xml— Profile opened while loading shows the cached name and avatar without edit actions, then the full profilering-choice-rows.xml— Pubky Ring rows show at once with a spinner each until their profile loads; adopting spins only the tapped row and disables the restManual Tests
Automated Checks
PubkyStoreTest.kt— the cached profile ownerPaykitSdkServiceTest.kt— public reads and locked work do not wait for each other, reads are capped at six, a read with no SDK builds it under the lock, activation does not wait for the refresh, and an approval waits for a refresh activation startedPubkyServiceTest.kt— cancelling a public read cancels the underlying callPubkyIdentityRepublishTest.kt— approvals still wait for the identity refreshPubkyRepoTest.kt— stale profile loads are dropped after a write, the retry policy per caller, the adopt handoff and the contact import reusePubkyChoiceViewModelTest.kt— rows show before lookups finish, each row's spinner clears whether its lookup finds a profile, finds nothing or fails, and lookups continue after a failed adoptionProfileViewModelTest.kt— cached header only while loading and only for the current key, refresh on open, and one more load when a running load failsPubkyImageFetcherTest.kt— disk cache hits, no writes after a clear or for a failed download, and disk errors falling back to the network