fix(auth): make _notifyAllSubscribers non-blocking to prevent callback deadlocks#2016
Open
7ttp wants to merge 1 commit intosupabase:masterfrom
Open
fix(auth): make _notifyAllSubscribers non-blocking to prevent callback deadlocks#20167ttp wants to merge 1 commit intosupabase:masterfrom
7ttp wants to merge 1 commit intosupabase:masterfrom
Conversation
|
@7ttp Thanks for the PR |
Contributor
|
This PR will re-introduce the SSR OAuth bug that #2039 fixes. The SSR callback is async: client.auth.onAuthStateChange(async (event) => {
await applyServerStorage({...})
}) With this PRs non-blocking approach, applyServerStorage won't complete before serverless functions return, causing the same cookie-setting failure as the original setTimeout issue. We need a different solution that:
Potential approaches:
|
Contributor
Author
|
hey @mandarini almost forgot about this, |
Contributor
|
let's park it for now @7ttp ! I want to see how the internal discussion goes!! Thank you for ALL the help!!!!! 💚 |
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.
Summary
Makes
_notifyAllSubscribersnon-blocking by removingawaiton subscriber callbacks. This prevents deadlocks when callbacks perform async operations that require the auth lock.Problem
When a subscriber callback in
onAuthStateChangeperforms async operations likegetUser()orgetSession(), a deadlock occurs:signInWithPassword) acquires lock via_acquireLock()await _notifyAllSubscribers()which awaits all callbacksgetUser()which tries to acquire the same lockgetUser()to wait for current operationgetUser()→ waits for lock → waits for callbackSolution
Remove
async/awaitfrom_notifyAllSubscribers. Callbacks fire synchronously, async callbacks run in background without blocking. Errors still caught and logged.Related
setTimeoutwrapper at one call site (exchangeCodeForSession). This fix makes_notifyAllSubscribersitself non-blocking, fixing all 20+ call sites.onAuthStateChangedoes not trigger intermittently, andrefreshSession/getUserhangs indefinitely supabase#41968