mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-12 04:04:26 +00:00
fix: snapshot patterns for refresh-tracker and metadata URLs
Two related lock-free snapshot refactors addressing the remaining
post-v1.0.16 code-review findings.
1. refreshAttemptTracker: per-field atomic.Load/Store -> atomic.Value
snapshot of *attemptState (refresh_coordinator.go).
Previously each tracker held five independently-atomic fields. The
cooldown-exit reset wrote cooldownEndNano = 0 first, then separately
stored attempts = 1 and windowStartNano = now. A concurrent
isInCooldown call could observe cooldownEndNano = 0 (reset just
completed) with attempts still at MaxRefreshAttempts, immediately
triggering a fresh cooldown — a benign double-trigger race that
nonetheless meant the state machine had observable intermediate
states.
New design: state is a *attemptState (immutable) published via
atomic.Value. All transitions (record/success/failure/window-reset/
cooldown-enter/cooldown-exit) go through mutateState, which runs a
CAS loop: load current snapshot -> construct fresh snapshot ->
CompareAndSwap. Either the entire new state publishes or none of
it does — no intermediate visibility, no cross-field race.
Under Yaegi this collapses 3-5 per-field atomic dispatches into one
atomic.Value.Load on the read path. Write paths pay an extra
allocation for the new snapshot but avoid the cross-field hazard.
2. MetadataSnapshot: hot-path readers use atomic.Value instead of
metadataMu.RLock (middleware.go, types.go, main.go, utilities.go).
middleware.ServeHTTP previously took metadataMu.RLock on every
non-bypass request to read the single field issuerURL. Under Yaegi
each RLock acquisition costs 1-5ms of interpreter dispatch.
updateMetadataEndpoints now also publishes an immutable
*MetadataSnapshot via atomic.Value; the hot-path reader loads it
in one op via t.metadataSnap(). Falls back to the legacy
metadataMu.RLock pattern when the snapshot is unpublished (some
test setups initialize the struct fields directly without going
through updateMetadataEndpoints).
Less-frequent callers (helpers, logout, token_introspection) still
take metadataMu.RLock and are unchanged. The snapshot strictly
subsets the metadataMu-protected fields, so those readers see
identical data.
Note on atomic.Pointer[T]: this would have been the cleaner type but
yaegi v0.16.1's stdlib (used by traefik:v3.7.1) exposes only the
legacy unsafe.Pointer-based atomic primitives — no generic Pointer[T].
atomic.Value provides the same semantics via interface{} + type assert.
All tests pass with -race; golangci-lint clean.
This commit is contained in:
@@ -517,6 +517,19 @@ func (t *TraefikOidc) updateMetadataEndpoints(metadata *ProviderMetadata) {
|
||||
introspectionURL := t.introspectionURL
|
||||
registrationURL := t.registrationURL
|
||||
|
||||
// Publish the read-mostly URL bundle atomically. Hot-path readers Load
|
||||
// this directly instead of acquiring metadataMu.RLock per request.
|
||||
t.metadataSnapshot.Store(&MetadataSnapshot{
|
||||
IssuerURL: metadata.Issuer,
|
||||
JWKSURL: metadata.JWKSURL,
|
||||
TokenURL: metadata.TokenURL,
|
||||
AuthURL: metadata.AuthURL,
|
||||
RevocationURL: metadata.RevokeURL,
|
||||
EndSessionURL: metadata.EndSessionURL,
|
||||
IntrospectionURL: metadata.IntrospectionURL,
|
||||
RegistrationURL: metadata.RegistrationURL,
|
||||
})
|
||||
|
||||
t.metadataMu.Unlock()
|
||||
|
||||
// Log introspection endpoint availability for opaque token support
|
||||
|
||||
Reference in New Issue
Block a user