Fix 8 race condition bugs in internal-session.js#3084
Open
lifeart wants to merge 2 commits intomainmatter:masterfrom
Open
Fix 8 race condition bugs in internal-session.js#3084lifeart wants to merge 2 commits intomainmatter:masterfrom
lifeart wants to merge 2 commits intomainmatter:masterfrom
Conversation
…sion Address 8 race condition bugs where `this.authenticator` can be null during async operations (token refresh, multi-tab sync, logout): 1. _lookupAuthenticator() — return null for falsy authenticator name 2. invalidate() — guard against null/missing authenticator 3. restore() — guard authenticator lookup with try/catch and null check 4. _bindToStoreEvents() — guard authenticator lookup with try/catch 5. _bindToAuthenticatorEvents() — remove old listeners before adding new 6. _clear() — unbind authenticator events before clearing session state 7. _updateStore() — prevent persisting without authenticator key 8. _onSessionDataUpdated() — guard against null authenticator Closes mainmatter#3083 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ve test specificity - Add debug() logging in _clear() and _bindToAuthenticatorEvents() catch blocks that previously swallowed errors silently - Add debug() log in _updateStore() when skipping persist due to empty authenticator - Improve _bindToAuthenticatorEvents test to also assert listeners are not added - Strengthen invalidate race-condition tests with precondition checks and assert.step/verifySteps pattern instead of broad try/catch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
Fixes 8 race condition bugs in
internal-session.jswherethis.authenticatorcan becomenullduring asynchronous operations such as token refresh, multi-tab session sync, and concurrent logout flows.Closes #3083
Changes
_lookupAuthenticator()— Returnsnullearly when the authenticator name is falsy, instead of attempting a container lookup with an invalid key.invalidate()— Guards againstthis.authenticatorbeingnullor the lookup failing. Falls back to_clear(true)directly so the session still gets properly invalidated.restore()— Wraps the authenticator lookup in try/catch and checks for null. If the authenticator can't be found, logs a debug message and clears the session with the restored content._bindToStoreEvents()— Wraps the authenticator lookup in try/catch and checks for null. If lookup fails, resets_busyand clears the session with content._bindToAuthenticatorEvents()— Removes existing event listeners before adding new ones to prevent listener accumulation across rapid re-authentication cycles. Also guards against null authenticator._clear()— Unbinds authenticator events (sessionDataUpdated,sessionDataInvalidated) before clearing session state. This prevents stale listeners from firing after the session has been invalidated._updateStore()— Returns a resolved promise whenisAuthenticatedis true butthis.authenticatoris empty, preventing the store from persisting session data without an authenticator key._onSessionDataUpdated()— Returns early whenthis.authenticatoris null, preventing_setup()from being called with a null authenticator during concurrent data update events.Test plan
internal-session-test.js🤖 Generated with Claude Code