fix(locator): keep users online while active bindings remain - #274
Merged
Conversation
- preserve authoritative registration identity on locator results - gate every binding-removal event on the final active binding - serialize locator mutations with local registration events - cover stale, fresh, concurrent, and wildcard registration paths
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
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.
Problem
The SIP registrar removes a binding through several independent paths: an
explicit
REGISTERwithexpires=0, a transport close (e.g. a browser thatcloses its WebSocket without unregistering), the periodic expired-registration
sweep, and lookup-time cleanup in the DB-backed locator.
A single registered identity (username + realm) may hold multiple active
bindings at once — for example, when a client reconnects and registers a fresh
Contact before the previous Contact expires or its transport is torn down.
Removing any one binding used to publish
LocatorEvent::Offline/Unregisteredfor the removed bindings directly. A stale binding could therefore overwrite the
fresh registration state and mark a still-registered user as offline, corrupting
downstream presence and agent state.
Change
Location(
registered_username,registered_realm), populated on register and restoredfrom the DB model, so each removal path knows which identity a binding belongs
to.
locations_without_active_bindingshelper re-checkshas_active_bindingsperidentity and only reports offline when no binding remains.
LocatorEventLock, so a concurrent register/unregister cannot race thecheck-then-publish pattern into emitting a stale event.
DbLocatorcleanup to compare-and-delete: an expired snapshot is removedonly when
id + last_modified + expiresstill match, inside a transaction, withsurvivors re-verified. This keeps a concurrent
REGISTER— including one fromanother process that cannot share the local mutex — from being deleted by a sweep
after it refreshed the same row.
Tests
Regression tests cover stale/fresh, concurrent, and wildcard registration paths,
asserting that removing an expired binding while a fresher binding for the same
identity remains does not report the user offline and keeps the fresh binding
routable.