Move locking into getToken()

Author: jacekradko

.changeset/fix-token-refresh-race-condition.md

@@ -2,5 +2,13 @@
 "@clerk/clerk-js": patch
 ---
 
-Fix race condition where multiple browser tabs could fetch session tokens simultaneously. The `refreshTokenOnFocus` handler now uses the same cross-tab lock as the session token poller, preventing duplicate API calls when switching between tabs or when focus events fire while another tab is already refreshing the token.
+Fix race condition where multiple browser tabs could fetch session tokens simultaneously. 
 
+Key changes:
+- `getToken()` now uses a cross-tab lock (via Web Locks API or localStorage fallback) to coordinate token refresh operations
+- Per-tokenId locking allows different token types (different orgs, JWT templates) to be fetched in parallel while preventing duplicates for the same token
+- Double-checked locking pattern: cache is checked before and after acquiring the lock, so tabs that wait will find the token already cached by the tab that fetched it
+- Graceful timeout handling: if lock acquisition times out (~5 seconds), the operation proceeds in degraded mode rather than failing
+- Removed redundant lock from SessionCookiePoller since coordination is now handled within `getToken()` itself
+
+This ensures all callers of `getToken()` (pollers, focus handlers, user code) automatically benefit from cross-tab coordination.

packages/clerk-js/src/core/auth/SessionCookiePoller.ts

@@ -1,48 +1,27 @@
 import { createWorkerTimers } from '@clerk/shared/workerTimers';
 
-import type { SafeLockReturn } from './safeLock';
-import { SafeLock } from './safeLock';
-
-export const REFRESH_SESSION_TOKEN_LOCK_KEY = 'clerk.lock.refreshSessionToken';
 const INTERVAL_IN_MS = 5 * 1_000;
 
 /**
- * Polls for session token refresh at regular intervals with cross-tab coordination.
- *
- * @example
- * ```typescript
- * // Create a shared lock for coordination with focus handlers
- * const sharedLock = SafeLock(REFRESH_SESSION_TOKEN_LOCK_KEY);
- *
- * // Poller uses the shared lock
- * const poller = new SessionCookiePoller(sharedLock);
- * poller.startPollingForSessionToken(() => refreshToken());
+ * Polls for session token refresh at regular intervals.
  *
- * // Focus handler can use the same lock to prevent races
- * window.addEventListener('focus', () => {
- *   sharedLock.acquireLockAndRun(() => refreshToken());
- * });
- * ```
+ * Note: Cross-tab coordination is handled within Session.getToken() itself,
+ * so this poller simply triggers the refresh callback without additional locking.
  */
 export class SessionCookiePoller {
-  private lock: SafeLockReturn;
   private workerTimers = createWorkerTimers();
   private timerId: ReturnType<typeof this.workerTimers.setInterval> | null = null;
   // Disallows for multiple `startPollingForSessionToken()` calls before `callback` is executed.
   private initiated = false;
 
-  constructor(lock?: SafeLockReturn) {
-    this.lock = lock ?? SafeLock(REFRESH_SESSION_TOKEN_LOCK_KEY);
-  }
-
   public startPollingForSessionToken(cb: () => Promise<unknown>): void {
     if (this.timerId || this.initiated) {
       return;
     }
 
     const run = async () => {
       this.initiated = true;
-      await this.lock.acquireLockAndRun(cb);
+      await cb();
       this.timerId = this.workerTimers.setTimeout(run, INTERVAL_IN_MS);
     };
 

packages/clerk-js/src/core/auth/__tests__/SessionCookiePoller.test.ts

@@ -1,6 +1,5 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
-import type { SafeLockReturn } from '../safeLock';
 import { SessionCookiePoller } from '../SessionCookiePoller';
 
 describe('SessionCookiePoller', () => {
@@ -13,132 +12,129 @@ describe('SessionCookiePoller', () => {
     vi.restoreAllMocks();
   });
 
-  describe('shared lock coordination', () => {
-    it('accepts an external lock for coordination with other components', () => {
-      const sharedLock: SafeLockReturn = {
-        acquireLockAndRun: vi.fn().mockResolvedValue(undefined),
-      };
-
-      const poller = new SessionCookiePoller(sharedLock);
+  describe('startPollingForSessionToken', () => {
+    it('executes callback immediately on start', async () => {
+      const poller = new SessionCookiePoller();
       const callback = vi.fn().mockResolvedValue(undefined);
 
       poller.startPollingForSessionToken(callback);
 
-      // Verify the shared lock is used
-      expect(sharedLock.acquireLockAndRun).toHaveBeenCalledWith(callback);
+      // Flush microtasks to let the async run() execute
+      await Promise.resolve();
+
+      expect(callback).toHaveBeenCalledTimes(1);
 
       poller.stopPollingForSessionToken();
     });
 
-    it('creates internal lock when none provided (backward compatible)', () => {
-      // Should not throw when no lock is provided
+    it('prevents multiple concurrent polling sessions', async () => {
       const poller = new SessionCookiePoller();
-      expect(poller).toBeInstanceOf(SessionCookiePoller);
-    });
-
-    it('enables focus handler and poller to share the same lock', () => {
-      // This test demonstrates the shared lock pattern used in AuthCookieService
-      const sharedLock: SafeLockReturn = {
-        acquireLockAndRun: vi.fn().mockImplementation(async (cb: () => Promise<unknown>) => {
-          return cb();
-        }),
-      };
-
-      const poller = new SessionCookiePoller(sharedLock);
-      const pollerCallback = vi.fn().mockResolvedValue('poller-result');
+      const callback = vi.fn().mockResolvedValue(undefined);
 
-      // Poller uses the shared lock
-      poller.startPollingForSessionToken(pollerCallback);
+      poller.startPollingForSessionToken(callback);
+      poller.startPollingForSessionToken(callback); // Second call should be ignored
 
-      // Simulate focus handler also using the shared lock (like AuthCookieService does)
-      const focusCallback = vi.fn().mockResolvedValue('focus-result');
-      void sharedLock.acquireLockAndRun(focusCallback);
+      await Promise.resolve();
 
-      // Both should use the same lock instance
-      expect(sharedLock.acquireLockAndRun).toHaveBeenCalledTimes(2);
-      expect(sharedLock.acquireLockAndRun).toHaveBeenCalledWith(pollerCallback);
-      expect(sharedLock.acquireLockAndRun).toHaveBeenCalledWith(focusCallback);
+      expect(callback).toHaveBeenCalledTimes(1);
 
       poller.stopPollingForSessionToken();
     });
   });
 
-  describe('startPollingForSessionToken', () => {
-    it('executes callback immediately on start', () => {
-      const sharedLock: SafeLockReturn = {
-        acquireLockAndRun: vi.fn().mockResolvedValue(undefined),
-      };
-
-      const poller = new SessionCookiePoller(sharedLock);
+  describe('stopPollingForSessionToken', () => {
+    it('stops polling when called', async () => {
+      const poller = new SessionCookiePoller();
       const callback = vi.fn().mockResolvedValue(undefined);
 
       poller.startPollingForSessionToken(callback);
+      await Promise.resolve();
 
-      expect(sharedLock.acquireLockAndRun).toHaveBeenCalledWith(callback);
+      expect(callback).toHaveBeenCalledTimes(1);
 
       poller.stopPollingForSessionToken();
-    });
-
-    it('prevents multiple concurrent polling sessions', () => {
-      const sharedLock: SafeLockReturn = {
-        acquireLockAndRun: vi.fn().mockResolvedValue(undefined),
-      };
-
-      const poller = new SessionCookiePoller(sharedLock);
-      const callback = vi.fn().mockResolvedValue(undefined);
 
-      poller.startPollingForSessionToken(callback);
-      poller.startPollingForSessionToken(callback); // Second call should be ignored
-
-      expect(sharedLock.acquireLockAndRun).toHaveBeenCalledTimes(1);
+      // Advance time - callback should not be called again
+      await vi.advanceTimersByTimeAsync(10000);
 
-      poller.stopPollingForSessionToken();
+      expect(callback).toHaveBeenCalledTimes(1);
     });
-  });
 
-  describe('stopPollingForSessionToken', () => {
     it('allows restart after stop', async () => {
-      const sharedLock: SafeLockReturn = {
-        acquireLockAndRun: vi.fn().mockResolvedValue(undefined),
-      };
-
-      const poller = new SessionCookiePoller(sharedLock);
+      const poller = new SessionCookiePoller();
       const callback = vi.fn().mockResolvedValue(undefined);
 
       // Start and stop
       poller.startPollingForSessionToken(callback);
+      await Promise.resolve();
       poller.stopPollingForSessionToken();
 
-      // Clear mock to check restart
-      vi.mocked(sharedLock.acquireLockAndRun).mockClear();
+      expect(callback).toHaveBeenCalledTimes(1);
 
       // Should be able to start again
       poller.startPollingForSessionToken(callback);
-      expect(sharedLock.acquireLockAndRun).toHaveBeenCalledTimes(1);
+      await Promise.resolve();
+
+      expect(callback).toHaveBeenCalledTimes(2);
 
       poller.stopPollingForSessionToken();
     });
   });
 
   describe('polling interval', () => {
     it('schedules next poll after callback completes', async () => {
-      const sharedLock: SafeLockReturn = {
-        acquireLockAndRun: vi.fn().mockResolvedValue(undefined),
-      };
-
-      const poller = new SessionCookiePoller(sharedLock);
+      const poller = new SessionCookiePoller();
       const callback = vi.fn().mockResolvedValue(undefined);
 
       poller.startPollingForSessionToken(callback);
 
       // Initial call
-      expect(sharedLock.acquireLockAndRun).toHaveBeenCalledTimes(1);
+      await Promise.resolve();
+      expect(callback).toHaveBeenCalledTimes(1);
 
       // Wait for first interval (5 seconds)
       await vi.advanceTimersByTimeAsync(5000);
 
       // Should have scheduled another call
-      expect(sharedLock.acquireLockAndRun).toHaveBeenCalledTimes(2);
+      expect(callback).toHaveBeenCalledTimes(2);
+
+      // Another interval
+      await vi.advanceTimersByTimeAsync(5000);
+      expect(callback).toHaveBeenCalledTimes(3);
+
+      poller.stopPollingForSessionToken();
+    });
+
+    it('waits for callback to complete before scheduling next poll', async () => {
+      const poller = new SessionCookiePoller();
+
+      let resolveCallback: () => void;
+      const callbackPromise = new Promise<void>(resolve => {
+        resolveCallback = resolve;
+      });
+      const callback = vi.fn().mockReturnValue(callbackPromise);
+
+      poller.startPollingForSessionToken(callback);
+
+      // Let the first call start
+      await Promise.resolve();
+      expect(callback).toHaveBeenCalledTimes(1);
+
+      // Advance time while callback is still running - should NOT schedule next poll
+      // because the callback promise hasn't resolved yet
+      await vi.advanceTimersByTimeAsync(5000);
+
+      // Should still only be 1 call since previous call hasn't completed
+      expect(callback).toHaveBeenCalledTimes(1);
+
+      // Complete the callback
+      resolveCallback!();
+      await Promise.resolve();
+
+      // Now advance time for the next interval
+      await vi.advanceTimersByTimeAsync(5000);
+
+      expect(callback).toHaveBeenCalledTimes(2);
 
       poller.stopPollingForSessionToken();
     });

packages/clerk-js/src/core/auth/safeLock.ts

@@ -1,42 +1,18 @@
 import Lock from 'browser-tabs-lock';
 
-/**
- * Return type for SafeLock providing cross-tab lock coordination.
- */
-export interface SafeLockReturn {
-  /**
-   * Acquires a cross-tab lock and executes the callback while holding it.
-   * Other tabs attempting to acquire the same lock will wait until this callback completes.
-   *
-   * @param cb - Async callback to execute while holding the lock
-   * @returns The callback's return value, or `false` if lock acquisition times out
-   */
-  acquireLockAndRun: (cb: () => Promise<unknown>) => Promise<unknown>;
-}
+import { debugLogger } from '@/utils/debug';
+
+const LOCK_TIMEOUT_MS = 4999;
 
 /**
- * Creates a cross-tab lock mechanism for coordinating exclusive operations across browser tabs.
- *
- * This is used to prevent multiple tabs from performing the same operation simultaneously,
- * such as refreshing session tokens. When one tab holds the lock, other tabs will wait
- * until the lock is released before proceeding.
+ * Creates a cross-tab lock for coordinating exclusive operations across browser tabs.
  *
- * @param key - Shared identifier for the lock
- * @returns SafeLockReturn with acquireLockAndRun method
+ * Uses Web Locks API in secure contexts (HTTPS), falling back to browser-tabs-lock
+ * (localStorage-based) in non-secure contexts.
  *
- * @example
- * ```typescript
- * const tokenLock = SafeLock('clerk.lock.refreshToken');
- *
- * // In Tab 1:
- * await tokenLock.acquireLockAndRun(async () => {
- *   await refreshToken(); // Only one tab executes this at a time
- * });
- *
- * // Tab 2 will wait for Tab 1 to finish before executing its callback
- * ```
+ * @param key - Unique identifier for the lock (same key = same lock across all tabs)
  */
-export function SafeLock(key: string): SafeLockReturn {
+export function SafeLock(key: string) {
   const lock = new Lock();
 
   // Release any held locks when the tab is closing to prevent deadlocks
@@ -45,32 +21,41 @@ export function SafeLock(key: string): SafeLockReturn {
     await lock.releaseLock(key);
   });
 
-  const acquireLockAndRun = async (cb: () => Promise<unknown>) => {
+  /**
+   * Acquires the cross-tab lock and executes the callback while holding it.
+   * If lock acquisition fails or times out, executes the callback anyway (degraded mode)
+   * to ensure the operation completes rather than failing.
+   */
+  const acquireLockAndRun = async <T>(cb: () => Promise<T>): Promise<T> => {
     if ('locks' in navigator && isSecureContext) {
       const controller = new AbortController();
-      const lockTimeout = setTimeout(() => controller.abort(), 4999);
+      const lockTimeout = setTimeout(() => controller.abort(), LOCK_TIMEOUT_MS);
 
-      return await navigator.locks
-        .request(key, { signal: controller.signal }, async () => {
+      try {
+        return await navigator.locks.request(key, { signal: controller.signal }, async () => {
           clearTimeout(lockTimeout);
           return await cb();
-        })
-        .catch(() => {
-          // Lock request was aborted (timeout) or failed
-          // Return false to indicate lock was not acquired (matches browser-tabs-lock behavior)
-          return false;
         });
+      } catch {
+        // Lock request was aborted (timeout) or failed
+        // Execute callback anyway in degraded mode to ensure operation completes
+        debugLogger.warn('Lock acquisition timed out, proceeding without lock (degraded mode)', { key }, 'safeLock');
+        return await cb();
+      }
     }
 
-    if (await lock.acquireLock(key, 5000)) {
+    // Fallback for non-secure contexts using localStorage-based locking
+    if (await lock.acquireLock(key, LOCK_TIMEOUT_MS + 1)) {
       try {
         return await cb();
       } finally {
         await lock.releaseLock(key);
       }
     }
 
-    return false;
+    // Lock acquisition timed out - execute callback anyway in degraded mode
+    debugLogger.warn('Lock acquisition timed out, proceeding without lock (degraded mode)', { key }, 'safeLock');
+    return await cb();
   };
 
   return { acquireLockAndRun };