fix(auth): make _notifyAllSubscribers non-blocking to prevent callback deadlocks
Summary
Makes _notifyAllSubscribers non-blocking by removing await on subscriber callbacks. This prevents deadlocks when callbacks perform async operations that require the auth lock.
Problem
When a subscriber callback in onAuthStateChange performs async operations like getUser() or getSession(), a deadlock occurs:
- Auth method (e.g.,
signInWithPassword) acquires lock via_acquireLock() - Inside the lock, it calls
await _notifyAllSubscribers()which awaits all callbacks - Subscriber callback calls
getUser()which tries to acquire the same lock - Lock queues
getUser()to wait for current operation - Current operation waits for callback to complete
-
Deadlock: callback waits for
getUser()→ waits for lock → waits for callback
Solution
Remove async/await from _notifyAllSubscribers. Callbacks fire synchronously, async callbacks run in background without blocking. Errors still caught and logged.
Related
Extends #2014 - that PR added setTimeout wrapper at one call site (exchangeCodeForSession). This fix makes _notifyAllSubscribers itself non-blocking, fixing all 20+ call sites.
closes https://github.com/supabase/supabase-js/issues/2013
Thanks @simplysparsh for the initial finding.
coverage: 81.017% (+0.02%) from 80.997% when pulling 60e2a3dedc951d813cc4beee6b5fd8f455de7342 on 7ttp:fix/auth-non-blocking-subscriber-notifications into 09aa10628b00cbdf65ace0f9e8e79237e7c0c1dc on supabase:master.
@7ttp Thanks for the PR @mandarini @grdsdev Would be great if you can review this.
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:
- Prevents deadlocks when callbacks call
getUser()orgetSession() - Ensures critical async operations (like cookie-setting) complete before returning
- Works in both serverless and long-running server contexts
Potential approaches:
- Detect if callback is async and warn/error if it awaits other auth methods
- Document that callbacks MUST NOT await auth methods
- Refactor locking mechanism to allow reentrant calls during notifications
- Make cookie-setting synchronous (queue writes instead of awaiting)
hey @mandarini almost forgot about this, was curious if this aswell is being discussed internally (in the recent lock related discussion) ? or should i continue with the suggested approaches? 😅💚
let's park it for now @7ttp ! I want to see how the internal discussion goes!! Thank you for ALL the help!!!!! 💚