fix(server): publish MultiService user table atomically - #6
Open
CloudPassenger wants to merge 1 commit into
Open
fix(server): publish MultiService user table atomically#6CloudPassenger wants to merge 1 commit into
CloudPassenger wants to merge 1 commit into
Conversation
MultiService[U] stored the user table as a plain map that UpdateUsers replaced while the authentication path read it concurrently. Every request on both the fresh-connection and session-reuse paths reads the table, so concurrent UpdateUsers calls raced with map reads and could crash the process with a concurrent map read/write panic. Replace the field with atomic.Pointer[map[string]U]. UpdateUsers keeps building a fully validated table before publishing it, so failed validation never modifies the published state; readers load one immutable snapshot per lookup without locking. Validation errors and their order are unchanged, authenticate still returns snell.ErrBadUserKey on miss (also before the first successful UpdateUsers), and the snell.Service interface assertions are intact. Add in-package regression tests driving UpdateUsers against authenticate concurrently; they fail under -race before this change.
CloudPassenger
force-pushed
the
fix/multiservice-users-race
branch
from
August 24, 2026 12:29
1a8948b to
c8e4502
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MultiService[U]keeps the user table in a plain map thatUpdateUsersreplaces while the authentication path reads it. Since every request reads the table (including on session-reuse connections), concurrentUpdateUserscalls race with map reads and can crash the process with a fatal concurrent map read/write panic.Upstream only calls
UpdateUsersonce at construction, which is why this has stayed latent, but it is an exported API intended for runtime user updates.The fix replaces the field with
atomic.Pointer[map[string]U]:UpdateUsersstill builds and validates a full replacement table before publishing it, and readers load one immutable snapshot per lookup without locking. Validation errors and their order are unchanged; failed validation never touches the published state.Both changes come with an in-package
-raceregression test that drivesUpdateUsersagainstauthenticateconcurrently; it fails before the patch and passes after.This bug was tracked down under high load with the help of an AI agent (ox-alpha), which also assisted in implementing the fix.