mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
fix(blossom): close the read-auth single-flight gap a fast signer slips through
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011APd48WJ5pZVK4Ltpj3YpL
This commit is contained in:
+33
@@ -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 {
|
private companion object {
|
||||||
// Matches OkHttpClientFactory's maxRequestsPerHost: the worst realistic
|
// Matches OkHttpClientFactory's maxRequestsPerHost: the worst realistic
|
||||||
// burst is one gated host filling every per-host dispatcher slot.
|
// 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.
|
// How long one signature takes in the concurrency test.
|
||||||
const val SIGN_MS = 300L
|
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
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+12
-1
@@ -103,7 +103,9 @@ class BlossomReadAuthTokenProvider(
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns the in-flight signature for [host], starting one if this caller
|
* 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
|
* Leader/follower over [ConcurrentHashMap.putIfAbsent] rather than
|
||||||
* `computeIfAbsent`: the completion handler removes the map entry, and a job
|
* `computeIfAbsent`: the completion handler removes the map entry, and a job
|
||||||
@@ -113,6 +115,15 @@ class BlossomReadAuthTokenProvider(
|
|||||||
private fun signOnce(host: String): CompletableDeferred<String?>? {
|
private fun signOnce(host: String): CompletableDeferred<String?>? {
|
||||||
inFlight[host]?.let { return it }
|
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<String?>(it) }
|
||||||
|
|
||||||
val signer = signerProvider() ?: return null
|
val signer = signerProvider() ?: return null
|
||||||
|
|
||||||
val fresh = CompletableDeferred<String?>()
|
val fresh = CompletableDeferred<String?>()
|
||||||
|
|||||||
Reference in New Issue
Block a user