Use one persisted storage lock

plans/versioned-localstorage-sessions.md

@@ -270,7 +270,7 @@ Network refreshes do not run while holding a Web Lock. A refresh captures the ge
 const baseRefreshJti = getRefreshJti(session.refreshJwt)
 const refreshed = await refreshSession()
 
-await navigator.locks.request(`bsky-session:${did}`, async () => {
+await navigator.locks.request('bsky-persisted-storage', async () => {
   const latest = readAccountFromLocalStorage(did)
   // Commit only if latest is active and still has baseRefreshJti.
 })
@@ -278,19 +278,11 @@ await navigator.locks.request(`bsky-session:${did}`, async () => {
 
 This avoids holding a cross-tab lock over network I/O. Multiple refresh requests may be in flight concurrently; server-side convergence and the generation-specific conditional commit make their results safe.
 
-All credential-changing commits use the same per-account lock:
+All persisted values share one localStorage blob, so every write uses the same persisted-storage lock. This includes successful refresh reconciliation, expiration, logout, account removal, login replacement, and unrelated preference writes. One root lock is sufficient: it serializes the complete read-modify-write operation, so nesting per-account locks would add no coordination.
-
-- successful refresh reconciliation;
-- expiration;
-- logout;
-- account removal; and
-- login replacing an existing account.
-
-Because all persisted values share one localStorage blob, every write also takes a root persisted-storage lock. The root lock prevents an unrelated preference write from racing the session read-modify-write; the per-account lock expresses credential ownership and gives account operations a consistent order.
 
 Feature-detect the Web Locks API. If `navigator.locks.request` is unavailable, run the operation without a lock rather than failing startup or session operations. Generation-specific conditional commits still reject stale work in this fallback mode, but localStorage read-modify-write is not fully serialized across tabs.
 
-If Tab A holds the locks, Tab B waits. Once Tab A writes and releases them, Tab B acquires them and rereads Tab A's new localStorage state before deciding what to commit.
+If Tab A holds the lock, Tab B waits. Once Tab A writes and releases it, Tab B acquires it and rereads Tab A's new localStorage state before deciding what to commit.
 
 The complete refresh flow is:
 
@@ -301,7 +293,7 @@ Capture base refresh jti
 Perform network refresh without a lock
    |
    v
-Acquire root + per-account Web Locks
+Acquire persisted-storage Web Lock
    |
    v
 Read authoritative localStorage state

src/state/persisted/__tests__/storage-lock.web-test.ts

@@ -1,6 +1,6 @@
 import {afterEach, describe, expect, it, jest} from '@jest/globals'
 
-import {runWithSessionCredentialLock} from '../session-lock.web'
+import {runWithPersistedStorageLock} from '../storage-lock.web'
 
 const originalNavigatorDescriptor = Object.getOwnPropertyDescriptor(
   globalThis,
@@ -22,7 +22,7 @@ afterEach(() => {
   }
 })
 
-describe('session credential locks on unsupported browsers', () => {
+describe('persisted storage lock on unsupported browsers', () => {
   it.each([
     ['navigator is unavailable', undefined],
     ['navigator.locks is unavailable', {}],
@@ -31,12 +31,23 @@ describe('session credential locks on unsupported browsers', () => {
     setNavigator(navigatorValue)
     const operation = jest.fn(() => 'result')
 
-    await expect(
+    await expect(runWithPersistedStorageLock({operation})).resolves.toBe(
-      runWithSessionCredentialLock({
+      'result',
-        accountDids: ['did:plc:example'],
+    )
-        operation,
-      }),
-    ).resolves.toBe('result')
     expect(operation).toHaveBeenCalledTimes(1)
   })
+
+  it('uses one root lock when Web Locks are available', async () => {
+    const request = jest.fn(
+      (_name: string, operation: () => string | Promise<string>) =>
+        Promise.resolve(operation()),
+    )
+    setNavigator({locks: {request}})
+
+    await expect(
+      runWithPersistedStorageLock({operation: () => 'result'}),
+    ).resolves.toBe('result')
+    expect(request).toHaveBeenCalledTimes(1)
+    expect(request.mock.calls[0]?.[0]).toBe('bsky-persisted-storage')
+  })
 })

src/state/persisted/index.web.ts

@@ -8,11 +8,11 @@ import {
   tryParse,
   tryStringify,
 } from '#/state/persisted/schema'
-import {runWithSessionCredentialLock} from './session-lock'
 import {
   applySessionUpdate,
   type SessionCredentialMutation,
 } from './session-merge'
+import {runWithPersistedStorageLock} from './storage-lock'
 import {type PersistedApi} from './types'
 import {normalizeData} from './util'
 
@@ -76,8 +76,7 @@ export function write<K extends keyof Schema>(
       "Session state must be written through '#/state/persisted/session'",
     )
   }
-  return runWithSessionCredentialLock({
+  return runWithPersistedStorageLock({
-    accountDids: [],
     operation: () => {
       const next = readFromStorage()
       if (next) {
@@ -145,8 +144,7 @@ export function onUpdate<K extends keyof Schema>(
 onUpdate satisfies PersistedApi['onUpdate']
 
 export function clearStorage(): Promise<void> {
-  return runWithSessionCredentialLock({
+  return runWithPersistedStorageLock({
-    accountDids: [],
     operation: () => {
       try {
         localStorage.removeItem(BSKY_STORAGE)

src/state/persisted/session-lock.web.ts

@@ -1,45 +0,0 @@
-const PERSISTED_STORAGE_LOCK = 'bsky-persisted-storage'
-const SESSION_LOCK_PREFIX = 'bsky-session:'
-
-export function runWithSessionCredentialLock<T>({
-  accountDids,
-  operation,
-}: {
-  accountDids: string[]
-  operation: () => T | Promise<T>
-}): Promise<T> {
-  const lockManager = getLockManager()
-  if (!lockManager) {
-    try {
-      return Promise.resolve(operation())
-    } catch (error) {
-      return Promise.reject(
-        error instanceof Error
-          ? error
-          : new Error('Session credential operation failed', {cause: error}),
-      )
-    }
-  }
-
-  const accountLockNames = [...new Set(accountDids)]
-    .sort()
-    .map(did => `${SESSION_LOCK_PREFIX}${did}`)
-  /* All values share one localStorage blob, so every write also takes its lock. */
-  const lockNames = [PERSISTED_STORAGE_LOCK, ...accountLockNames]
-
-  const run = (index: number): Promise<T> => {
-    const lockName = lockNames[index]
-    if (!lockName) return Promise.resolve(operation())
-    return lockManager.request(lockName, () => run(index + 1))
-  }
-
-  return run(0)
-}
-
-function getLockManager(): LockManager | undefined {
-  if (typeof navigator === 'undefined' || !('locks' in navigator)) {
-    return undefined
-  }
-  const lockManager = navigator.locks
-  return typeof lockManager?.request === 'function' ? lockManager : undefined
-}

src/state/persisted/session.ts

@@ -1,9 +1,9 @@
 import * as persisted from './index'
 import {type Schema} from './schema'
-import {runWithSessionCredentialLock} from './session-lock'
 import {type SessionCredentialMutation} from './session-merge'
 
 export type {SessionCredentialMutation} from './session-merge'
+export {runWithPersistedStorageLock as runWithCredentialLock} from './storage-lock'
 
 export function read(): Schema['session'] {
   return persisted.get('session')
@@ -28,16 +28,6 @@ export function write({
   })
 }
 
-export function runWithCredentialLock<T>({
-  accountDids,
-  operation,
-}: {
-  accountDids: string[]
-  operation: () => T | Promise<T>
-}): Promise<T> {
-  return runWithSessionCredentialLock({accountDids, operation})
-}
-
 export function onUpdate(
   callback: (session: Schema['session']) => void,
 ): () => void {

src/state/persisted/storage-lock.ts

@@ -1,18 +1,15 @@
-export function runWithSessionCredentialLock<T>({
+export function runWithPersistedStorageLock<T>({
-  accountDids,
   operation,
 }: {
-  accountDids: string[]
   operation: () => T | Promise<T>
 }): Promise<T> {
-  void accountDids
   try {
     return Promise.resolve(operation())
   } catch (error) {
     return Promise.reject(
       error instanceof Error
         ? error
-        : new Error('Session credential operation failed', {cause: error}),
+        : new Error('Persisted storage operation failed', {cause: error}),
     )
   }
 }

src/state/persisted/storage-lock.web.ts

@@ -0,0 +1,30 @@
+const PERSISTED_STORAGE_LOCK = 'bsky-persisted-storage'
+
+export function runWithPersistedStorageLock<T>({
+  operation,
+}: {
+  operation: () => T | Promise<T>
+}): Promise<T> {
+  const lockManager = getLockManager()
+  if (!lockManager) {
+    try {
+      return Promise.resolve(operation())
+    } catch (error) {
+      return Promise.reject(
+        error instanceof Error
+          ? error
+          : new Error('Persisted storage operation failed', {cause: error}),
+      )
+    }
+  }
+
+  return lockManager.request(PERSISTED_STORAGE_LOCK, operation)
+}
+
+function getLockManager(): LockManager | undefined {
+  if (typeof navigator === 'undefined' || !('locks' in navigator)) {
+    return undefined
+  }
+  const lockManager = navigator.locks
+  return typeof lockManager?.request === 'function' ? lockManager : undefined
+}

src/state/session/index.tsx

@@ -168,7 +168,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
           : sessionData?.refreshJwt
 
       return persistedSession.runWithCredentialLock({
-        accountDids: [accountDid],
         operation: async () => {
           /*
            * Only the live bundle may reset the expiry-rescue bookkeeping: a stale
@@ -355,7 +354,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
         return
       }
       await persistedSession.runWithCredentialLock({
-        accountDids: [account.did],
         operation: () =>
           store.dispatch(
             {
@@ -399,7 +397,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
         return
       }
       await persistedSession.runWithCredentialLock({
-        accountDids: [account.did],
         operation: () =>
           store.dispatch(
             {
@@ -441,7 +438,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
       if (accountDid) {
         void persistedSession
           .runWithCredentialLock({
-            accountDids: [accountDid],
             operation: () =>
               store.dispatch({type: 'logged-out-current-account', accountDid}, [
                 {type: 'logout', accountDid},
@@ -488,7 +484,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
       ]
       void persistedSession
         .runWithCredentialLock({
-          accountDids,
           operation: () =>
             store.dispatch(
               {type: 'logged-out-every-account'},
@@ -555,7 +550,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
         return
       }
       const committedSession = await persistedSession.runWithCredentialLock({
-        accountDids: [account.did],
         operation: () =>
           store.dispatch(
             {
@@ -625,7 +619,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
     const data = await bundle.pdsClient.call(com.atproto.server.getSession, {})
     if (signal.aborted) return
     await persistedSession.runWithCredentialLock({
-      accountDids: [data.did],
       operation: () =>
         store.dispatch({
           type: 'partial-refresh-session',
@@ -713,7 +706,6 @@ export function Provider({children}: React.PropsWithChildren<{}>) {
       cancelPendingTask()
       void persistedSession
         .runWithCredentialLock({
-          accountDids: [account.did],
           operation: () =>
             store.dispatch(
               {