From 6dc631e85da6d2dbe499f77fbf03cdebf0aa4857 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 12 Sep 2026 14:50:15 +0000 Subject: [PATCH] fix(blossom): close the read-auth single-flight gap a fast signer slips through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BlossomReadAuthTokenProvider.header() reads the token cache, then signOnce() reads the in-flight map — two separate reads. A leader caches its token before retiring its in-flight entry, so a caller sitting between those two reads sees an empty cache (its read came first) and an empty in-flight map (the leader already finished), and signs a second token for the same host. A 300ms test signer never opens that window, which is why the provider's own concurrency test missed it. A local in-process key signs in microseconds, so BlossomReadAuthFetcherTest.aBurstOf401sSharesOneSignature — 16 fetchers that all 401 and all retry — hit it and intermittently saw two distinct tokens. signOnce() now takes a second look at the cache once it finds no in-flight entry: an absent entry proves the leader's cache write is already visible, so the straggler reuses that token instead of starting another signature. Covered by a new aFastSignerStillSharesOneSignature, which runs the 16-caller burst against an instant signer over many rounds — the existing test's slow signer cannot reach the window. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011APd48WJ5pZVK4Ltpj3YpL --- .../BlossomReadAuthTokenProviderTest.kt | 33 +++++++++++++++++++ .../http/BlossomReadAuthTokenProvider.kt | 13 +++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProviderTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProviderTest.kt index c235ee280a..5c1655434b 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProviderTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/BlossomReadAuthTokenProviderTest.kt @@ -218,6 +218,35 @@ class BlossomReadAuthTokenProviderTest { ) } + /** + * The same single-flight guarantee, but with a signature that returns almost + * immediately — an in-process [NostrSignerInternal], which is what the image + * path uses for a local key. + * + * A fast signature is the harder case: the leader can finish, cache its token + * and retire its in-flight entry while a straggler is still between its own + * cache miss and its look at the in-flight map. That straggler finds both + * empty, and must pick the just-minted token up instead of signing a second + * one. Run over many rounds because the window is only microseconds wide. + */ + @Test + fun aFastSignerStillSharesOneSignature() = + runBlocking { + repeat(ROUNDS) { round -> + val instant = DelayingTestSigner(delayMs = 0) + val provider = BlossomReadAuthTokenProvider({ instant }, scope) + + val results = + (1..CONCURRENT_CALLERS) + .map { async(Dispatchers.Default) { provider.header(host) } } + .awaitAll() + + assertEquals("round $round: one signature for $CONCURRENT_CALLERS callers", 1, instant.signatures) + assertEquals("round $round: every caller must get the same token", 1, results.toSet().size) + assertNotNull("round $round: token must be non-null", results.first()) + } + } + private companion object { // Matches OkHttpClientFactory's maxRequestsPerHost: the worst realistic // burst is one gated host filling every per-host dispatcher slot. @@ -225,5 +254,9 @@ class BlossomReadAuthTokenProviderTest { // How long one signature takes in the concurrency test. const val SIGN_MS = 300L + + // Rounds of the fast-signer burst. The leader-finished-early window is + // microseconds wide, so one round hits it only now and then. + const val ROUNDS = 200 } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt index e15935bfee..7c635ce442 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/service/http/BlossomReadAuthTokenProvider.kt @@ -103,7 +103,9 @@ class BlossomReadAuthTokenProvider( /** * Returns the in-flight signature for [host], starting one if this caller - * wins the race. Null when there is no signer to sign with. + * wins the race — or an already-completed one carrying the token a leader + * cached while this caller was on its way in. Null when there is no signer + * to sign with. * * Leader/follower over [ConcurrentHashMap.putIfAbsent] rather than * `computeIfAbsent`: the completion handler removes the map entry, and a job @@ -113,6 +115,15 @@ class BlossomReadAuthTokenProvider( private fun signOnce(host: String): CompletableDeferred? { inFlight[host]?.let { return it } + // Second look at the cache, because the caller's own miss happened before this + // read and a leader caches its token *before* retiring its [inFlight] entry — so + // an absent entry here means any token that leader minted is already visible. + // Without this look, a signature fast enough to finish inside that gap (a local + // key signs in microseconds) loses the single-flight guarantee: every straggler + // still between its cache miss and this read finds both empty and signs again, + // which is the N-signatures burst [inFlight] exists to collapse. + cachedHeader(host)?.let { return CompletableDeferred(it) } + val signer = signerProvider() ?: return null val fresh = CompletableDeferred()