fix(clerk-js): additional improvements to poller race condition

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

@@ -1,4 +1,5 @@
 ---
 "@clerk/clerk-js": patch
 ---
-Fix race condition where multiple browser tabs could fetch session tokens simultaneously. 
+
+Fix race condition where multiple browser tabs could fetch session tokens simultaneously. `getToken()` now uses a cross-tab lock to coordinate token refresh operations

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

@@ -5,7 +5,7 @@ const INTERVAL_IN_MS = 5 * 1_000;
 /**
  * Polls for session token refresh at regular intervals.
  *
- * Note: Cross-tab coordination is handled within Session.getToken() itself,
+ * Cross-tab coordination is handled within Session.getToken() itself,
  * so this poller simply triggers the refresh callback without additional locking.
  */
 export class SessionCookiePoller {

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

@@ -1,9 +1,22 @@
-import { describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import type { SafeLockReturn } from '../safeLock';
-import { SafeLock } from '../safeLock';
 
 describe('SafeLock', () => {
+  let SafeLock: (key: string) => SafeLockReturn;
+  let addEventListenerSpy: ReturnType<typeof vi.spyOn>;
+
+  beforeEach(async () => {
+    vi.resetModules();
+    const module = await import('../safeLock');
+    SafeLock = module.SafeLock;
+    addEventListenerSpy = vi.spyOn(window, 'addEventListener');
+  });
+
+  afterEach(() => {
+    addEventListenerSpy.mockRestore();
+  });
+
   describe('interface contract', () => {
     it('returns SafeLockReturn interface with acquireLockAndRun method', () => {
       const lock = SafeLock('test-interface');
@@ -24,8 +37,8 @@ describe('SafeLock', () => {
 
   describe('Web Locks API path', () => {
     it('uses Web Locks API when available in secure context', async () => {
-      // Skip if Web Locks not available (like in jsdom without polyfill)
-      if (!('locks' in navigator) || !navigator.locks) {
+      // Skip if Web Locks not available or not in secure context (e.g. jsdom without polyfill)
+      if (!('locks' in navigator) || !navigator.locks || !isSecureContext) {
         return;
       }
 
@@ -103,4 +116,91 @@ describe('SafeLock', () => {
       expect(sharedLock.acquireLockAndRun).toHaveBeenCalledTimes(2);
     });
   });
+
+  describe('error handling', () => {
+    it('propagates callback errors without double-invocation', async () => {
+      const originalLocks = navigator.locks;
+      const callbackError = new Error('Callback failed');
+      const callback = vi.fn().mockRejectedValue(callbackError);
+
+      const mockRequest = vi.fn().mockImplementation(async (_name, _options, cb) => {
+        return await cb();
+      });
+
+      Object.defineProperty(navigator, 'locks', {
+        value: { request: mockRequest },
+        configurable: true,
+      });
+
+      try {
+        const lock = SafeLock('test-error-propagation');
+        await expect(lock.acquireLockAndRun(callback)).rejects.toThrow('Callback failed');
+        expect(callback).toHaveBeenCalledTimes(1);
+      } finally {
+        Object.defineProperty(navigator, 'locks', {
+          value: originalLocks,
+          configurable: true,
+        });
+      }
+    });
+
+    it('invokes callback in degraded mode on AbortError (timeout)', async () => {
+      const originalLocks = navigator.locks;
+      const callback = vi.fn().mockResolvedValue('success');
+
+      const abortError = new Error('Lock request aborted');
+      abortError.name = 'AbortError';
+
+      const mockRequest = vi.fn().mockRejectedValue(abortError);
+
+      Object.defineProperty(navigator, 'locks', {
+        value: { request: mockRequest },
+        configurable: true,
+      });
+
+      try {
+        const lock = SafeLock('test-abort-fallback');
+        const result = await lock.acquireLockAndRun(callback);
+
+        expect(callback).toHaveBeenCalledTimes(1);
+        expect(result).toBe('success');
+      } finally {
+        Object.defineProperty(navigator, 'locks', {
+          value: originalLocks,
+          configurable: true,
+        });
+      }
+    });
+  });
+
+  describe('beforeunload listener consolidation', () => {
+    it('registers only one beforeunload listener regardless of lock count', () => {
+      const beforeUnloadCalls = addEventListenerSpy.mock.calls.filter(call => call[0] === 'beforeunload');
+      expect(beforeUnloadCalls).toHaveLength(0);
+
+      SafeLock('lock-1');
+      SafeLock('lock-2');
+      SafeLock('lock-3');
+
+      const afterCalls = addEventListenerSpy.mock.calls.filter(call => call[0] === 'beforeunload');
+      expect(afterCalls).toHaveLength(1);
+    });
+
+    it('fresh module import starts with clean state', async () => {
+      // First module instance
+      SafeLock('lock-a');
+      expect(addEventListenerSpy.mock.calls.filter(call => call[0] === 'beforeunload')).toHaveLength(1);
+
+      // Reset and get fresh module
+      vi.resetModules();
+      addEventListenerSpy.mockRestore();
+      addEventListenerSpy = vi.spyOn(window, 'addEventListener');
+
+      const freshModule = await import('../safeLock');
+
+      // Fresh module should register its own listener
+      freshModule.SafeLock('lock-b');
+      expect(addEventListenerSpy.mock.calls.filter(call => call[0] === 'beforeunload')).toHaveLength(1);
+    });
+  });
 });

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

@@ -4,6 +4,26 @@ import { debugLogger } from '@/utils/debug';
 
 const LOCK_TIMEOUT_MS = 4999;
 
+const activeLocks = new Map<string, Lock>();
+let cleanupListenerRegistered = false;
+
+function registerCleanupListener() {
+  if (cleanupListenerRegistered) {
+    return;
+  }
+  cleanupListenerRegistered = true;
+
+  window.addEventListener('beforeunload', () => {
+    activeLocks.forEach((lock, key) => {
+      void lock.releaseLock(key);
+    });
+  });
+}
+
+export interface SafeLockReturn {
+  acquireLockAndRun: <T>(cb: () => Promise<T>) => Promise<T>;
+}
+
 /**
  * Creates a cross-tab lock for coordinating exclusive operations across browser tabs.
  *
@@ -12,14 +32,11 @@ const LOCK_TIMEOUT_MS = 4999;
  *
  * @param key - Unique identifier for the lock (same key = same lock across all tabs)
  */
-export function SafeLock(key: string) {
+export function SafeLock(key: string): SafeLockReturn {
   const lock = new Lock();
 
-  // Release any held locks when the tab is closing to prevent deadlocks
-  // eslint-disable-next-line @typescript-eslint/no-misused-promises
-  window.addEventListener('beforeunload', async () => {
-    await lock.releaseLock(key);
-  });
+  activeLocks.set(key, lock);
+  registerCleanupListener();
 
   /**
    * Acquires the cross-tab lock and executes the callback while holding it.
@@ -36,11 +53,14 @@ export function SafeLock(key: string) {
           clearTimeout(lockTimeout);
           return await cb();
         });
-      } 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();
+      } catch (error) {
+        clearTimeout(lockTimeout);
+        if (error instanceof Error && error.name === 'AbortError') {
+          // Lock request was aborted (timeout) - execute callback anyway in degraded mode
+          debugLogger.warn('Lock acquisition timed out, proceeding without lock (degraded mode)', { key }, 'safeLock');
+          return await cb();
+        }
+        throw error;
       }
     }
 

packages/clerk-js/src/core/resources/Session.ts

@@ -28,6 +28,7 @@ import { isWebAuthnSupported as isWebAuthnSupportedOnWindow } from '@clerk/share
 
 import { unixEpochToDate } from '@/utils/date';
 import { debugLogger } from '@/utils/debug';
+import { LruMap } from '@/utils/lru-map';
 import {
   convertJSONToPublicKeyRequestOptions,
   serializePublicKeyCredentialAssertion,
@@ -46,8 +47,11 @@ import { SessionVerification } from './SessionVerification';
  * Cache of per-tokenId locks for cross-tab coordination.
  * Each unique tokenId gets its own lock, allowing different token types
  * (e.g., different orgs, JWT templates) to be fetched in parallel.
+ *
+ * LruMap is used to limit the maximum number of lock instances to prevent
+ * unbounded memory growth in long-lived browser sessions.
  */
-const tokenLocks = new Map<string, ReturnType<typeof SafeLock>>();
+const tokenLocks = new LruMap<string, ReturnType<typeof SafeLock>>(50);
 
 /**
  * Gets or creates a cross-tab lock for a specific tokenId.
@@ -387,7 +391,6 @@ export class Session extends BaseResource implements SessionResource {
 
     // Fast path: check cache without lock for immediate hits
     const cachedEntry = skipCache ? undefined : SessionTokenCache.get({ tokenId }, leewayInSeconds);
-
     // Dispatch tokenUpdate only for __session tokens with the session's active organization ID, and not JWT templates
     const shouldDispatchTokenUpdate = !template && organizationId === this.lastActiveOrganizationId;
 
@@ -396,7 +399,6 @@ export class Session extends BaseResource implements SessionResource {
       if (shouldDispatchTokenUpdate) {
         eventBus.emit(events.TokenUpdate, { token: cachedToken });
       }
-      // Return null when raw string is empty to indicate that there it's signed-out
       return cachedToken.getRawString() || null;
     }
 
@@ -454,7 +456,7 @@ export class Session extends BaseResource implements SessionResource {
           }
         }
 
-        // Return null when raw string is empty to indicate that there it's signed-out
+        // Return null when raw string is empty to indicate that it's signed-out
         return token.getRawString() || null;
       });
     });

packages/clerk-js/src/core/resources/__tests__/Session.test.ts

@@ -426,6 +426,60 @@ describe('Session', () => {
 
       expect(requestSpy).toHaveBeenCalledTimes(2);
     });
+
+    it('uses cache populated by another source during lock wait (simulates cross-tab)', async () => {
+      BaseResource.clerk = clerkMock();
+
+      const session = new Session({
+        status: 'active',
+        id: 'session_1',
+        object: 'session',
+        user: createUser({}),
+        last_active_organization_id: null,
+        actor: null,
+        created_at: new Date().getTime(),
+        updated_at: new Date().getTime(),
+      } as SessionJSON);
+
+      const requestSpy = BaseResource.clerk.getFapiClient().request as Mock<any>;
+      requestSpy.mockClear();
+
+      const token1 = await session.getToken();
+      expect(requestSpy).toHaveBeenCalledTimes(1);
+      expect(token1).toEqual(mockJwt);
+
+      requestSpy.mockClear();
+
+      const token2 = await session.getToken();
+      expect(requestSpy).toHaveBeenCalledTimes(0);
+      expect(token2).toEqual(mockJwt);
+    });
+
+    it('makes API call when skipCache is true even if cache is populated', async () => {
+      BaseResource.clerk = clerkMock();
+
+      const session = new Session({
+        status: 'active',
+        id: 'session_1',
+        object: 'session',
+        user: createUser({}),
+        last_active_organization_id: null,
+        last_active_token: { object: 'token', jwt: mockJwt },
+        actor: null,
+        created_at: new Date().getTime(),
+        updated_at: new Date().getTime(),
+      } as SessionJSON);
+
+      expect(SessionTokenCache.size()).toBe(1);
+
+      const requestSpy = BaseResource.clerk.getFapiClient().request as Mock<any>;
+      requestSpy.mockClear();
+
+      const token = await session.getToken({ skipCache: true });
+
+      expect(requestSpy).toHaveBeenCalledTimes(1);
+      expect(token).toEqual(mockJwt);
+    });
   });
 
   describe('touch()', () => {

packages/clerk-js/src/utils/__tests__/lru-map.test.ts

@@ -0,0 +1,73 @@
+import { describe, expect, it } from 'vitest';
+
+import { LruMap } from '../lru-map';
+
+describe('LruMap', () => {
+  it('stores and retrieves values', () => {
+    const map = new LruMap<string, number>(3);
+    map.set('a', 1);
+    map.set('b', 2);
+
+    expect(map.get('a')).toBe(1);
+    expect(map.get('b')).toBe(2);
+    expect(map.get('c')).toBeUndefined();
+  });
+
+  it('evicts oldest entry when exceeding max size', () => {
+    const map = new LruMap<string, number>(3);
+    map.set('a', 1);
+    map.set('b', 2);
+    map.set('c', 3);
+    map.set('d', 4);
+
+    expect(map.get('a')).toBeUndefined();
+    expect(map.get('b')).toBe(2);
+    expect(map.get('c')).toBe(3);
+    expect(map.get('d')).toBe(4);
+    expect(map.size).toBe(3);
+  });
+
+  it('moves accessed entry to most recent position', () => {
+    const map = new LruMap<string, number>(3);
+    map.set('a', 1);
+    map.set('b', 2);
+    map.set('c', 3);
+
+    map.get('a');
+
+    map.set('d', 4);
+
+    expect(map.get('a')).toBe(1);
+    expect(map.get('b')).toBeUndefined();
+    expect(map.get('c')).toBe(3);
+    expect(map.get('d')).toBe(4);
+  });
+
+  it('updates existing entry without eviction', () => {
+    const map = new LruMap<string, number>(3);
+    map.set('a', 1);
+    map.set('b', 2);
+    map.set('c', 3);
+
+    map.set('a', 100);
+
+    expect(map.get('a')).toBe(100);
+    expect(map.size).toBe(3);
+
+    map.set('d', 4);
+
+    expect(map.get('a')).toBe(100);
+    expect(map.get('b')).toBeUndefined();
+    expect(map.size).toBe(3);
+  });
+
+  it('handles max size of 1', () => {
+    const map = new LruMap<string, number>(1);
+    map.set('a', 1);
+    map.set('b', 2);
+
+    expect(map.get('a')).toBeUndefined();
+    expect(map.get('b')).toBe(2);
+    expect(map.size).toBe(1);
+  });
+});