fix: snapshot backgroundFetchSize before invoking user code

src/index.ts

@@ -45,6 +45,9 @@ export type Index = number & { [TYPE]: 'LRUCache Index' }
 const isPosInt = (n: unknown): n is PosInt =>
   !!n && n === Math.floor(n as number) && n > 0 && isFinite(n)
 
+const isNonNegativeInt = (n: unknown): n is number =>
+  typeof n === 'number' && (n === 0 || isPosInt(n))
+
 export type UintArray = Uint8Array | Uint16Array | Uint32Array
 export type NumberArray = UintArray | number[]
 
@@ -115,6 +118,7 @@ export type BackgroundFetch<V> = Promise<V | undefined> & {
   __returned: BackgroundFetch<V> | undefined
   __abortController: AbortController
   __staleWhileFetching: V | undefined
+  __size?: number
 }
 
 export type DisposeTask<K, V> = [
@@ -941,6 +945,8 @@ export namespace LRUCache {
      * replace. If not, then this value is used as its effective size.
      *
      * @default 1
+     *
+     * Must be a nonnegative integer if provided.
      */
     backgroundFetchSize?: number
 
@@ -1450,6 +1456,12 @@ export class LRUCache<K extends {}, V extends {}, FC = unknown> {
       perf,
     } = options
 
+    if (!isNonNegativeInt(backgroundFetchSize)) {
+      throw new TypeError(
+        'backgroundFetchSize must be a nonnegative integer',
+      )
+    }
+
     this.backgroundFetchSize = backgroundFetchSize
 
     if (perf !== undefined) {
@@ -1741,7 +1753,13 @@ export class LRUCache<K extends {}, V extends {}, FC = unknown> {
           // NB: this cannot occur if v.__staleWhileFetching is set,
           // because in that case, it would take on the size of the
           // existing entry that it temporarily replaces.
-          return this.backgroundFetchSize
+          const backgroundFetchSize = v.__size
+          if (!isNonNegativeInt(backgroundFetchSize)) {
+            throw new TypeError(
+              'backgroundFetchSize must be a nonnegative integer',
+            )
+          }
+          return backgroundFetchSize
         }
         if (sizeCalculation) {
           if (typeof sizeCalculation !== 'function') {
@@ -2498,6 +2516,19 @@ export class LRUCache<K extends {}, V extends {}, FC = unknown> {
       return v
     }
 
+    // Capture the provisional size before invoking user fetchMethod code.
+    // Accounting must consume this validated snapshot rather than re-reading
+    // the mutable public field after the synchronous Promise executor returns.
+    let backgroundFetchSize: number | undefined
+    if (index === undefined && this.#sizes !== undefined) {
+      backgroundFetchSize = this.backgroundFetchSize
+      if (!isNonNegativeInt(backgroundFetchSize)) {
+        throw new TypeError(
+          'backgroundFetchSize must be a nonnegative integer',
+        )
+      }
+    }
+
     const ac = new AbortController()
     const { signal } = options
     // when/if our AC signals, then stop listening to theirs.
@@ -2621,6 +2652,7 @@ export class LRUCache<K extends {}, V extends {}, FC = unknown> {
       __abortController: ac,
       __staleWhileFetching: v,
       __returned: undefined,
+      __size: backgroundFetchSize,
     })
 
     if (index === undefined) {

test/background-fetch-size.ts

@@ -5,6 +5,11 @@ const clock = t.clock
 clock.advance(1)
 clock.enter()
 
+const invalidBackgroundFetchSizeError = {
+  name: 'TypeError',
+  message: 'backgroundFetchSize must be a nonnegative integer',
+}
+
 t.test('background fetch size tests', async t => {
   const res: Record<number, (n: number) => void> = {}
   const c = new LRUCache<number, number>({
@@ -42,3 +47,158 @@ t.test('background fetch size tests', async t => {
   t.equal(c.calculatedSize, 10)
   await t.rejects(p3, new Error('evicted'))
 })
+
+t.test('backgroundFetchSize must be a nonnegative integer', t => {
+  const hostile = {
+    [Symbol.toPrimitive]() {
+      throw new Error('must not coerce backgroundFetchSize')
+    },
+  }
+  const invalid = [
+    ['negative', -1],
+    ['fractional', 1.5],
+    ['NaN', Number.NaN],
+    ['infinity', Number.POSITIVE_INFINITY],
+    ['string', '2'],
+    ['symbol', Symbol('2')],
+    ['hostile object', hostile],
+  ] as const
+
+  for (const [label, backgroundFetchSize] of invalid) {
+    t.throws(
+      () =>
+        new LRUCache({
+          max: 1,
+          backgroundFetchSize: backgroundFetchSize as unknown as number,
+        }),
+      invalidBackgroundFetchSizeError,
+      label,
+    )
+  }
+
+  t.doesNotThrow(() => new LRUCache({ max: 1, backgroundFetchSize: 0 }))
+  t.doesNotThrow(() => new LRUCache({ max: 1, backgroundFetchSize: 1 }))
+  t.end()
+})
+
+t.test('mutated size is validated before fetch dispatch', async t => {
+  let fetchCalls = 0
+  const c = new LRUCache<number, number>({
+    maxSize: 10,
+    sizeCalculation: () => 5,
+    fetchMethod: async key => {
+      fetchCalls++
+      return key
+    },
+  })
+
+  c.backgroundFetchSize = Symbol('2') as unknown as number
+  await t.rejects(c.fetch(1), invalidBackgroundFetchSizeError)
+  t.equal(fetchCalls, 0)
+  t.equal(c.size, 0)
+  t.equal(c.calculatedSize, 0)
+})
+
+t.test('fetch snapshots size before callback mutation', async t => {
+  const deferred = new Map<number, PromiseWithResolvers<number>>()
+  let fetchCalls = 0
+  let c: LRUCache<number, number>
+  c = new LRUCache<number, number>({
+    maxSize: 20,
+    sizeCalculation: () => 5,
+    backgroundFetchSize: 2,
+    fetchMethod: async key => {
+      fetchCalls++
+      if (key === 1) {
+        c.backgroundFetchSize = 4
+      }
+      const result = Promise.withResolvers<number>()
+      deferred.set(key, result)
+      return result.promise
+    },
+  })
+
+  const first = c.fetch(1)
+  const firstAgain = c.fetch(1)
+  t.equal(fetchCalls, 1)
+  t.equal(c.calculatedSize, 2)
+
+  const second = c.fetch(2)
+  t.equal(fetchCalls, 2)
+  t.equal(c.calculatedSize, 6)
+
+  deferred.get(1)?.resolve(1)
+  deferred.get(2)?.resolve(2)
+  t.same(await Promise.all([first, firstAgain, second]), [1, 1, 2])
+  t.equal(c.calculatedSize, 10)
+})
+
+t.test('mutated size is ignored without size tracking', async t => {
+  let fetchCalls = 0
+  const c = new LRUCache<number, number>({
+    max: 1,
+    fetchMethod: async key => {
+      fetchCalls++
+      return key
+    },
+  })
+
+  c.backgroundFetchSize = Symbol('2') as unknown as number
+  t.equal(await c.fetch(1), 1)
+  t.equal(fetchCalls, 1)
+  t.equal(c.size, 1)
+})
+
+t.test('mutated size is ignored for stale refresh', async t => {
+  const deferred = Promise.withResolvers<number>()
+  let fetchCalls = 0
+  const c = new LRUCache<number, number>({
+    maxSize: 10,
+    sizeCalculation: () => 5,
+    ttl: 10,
+    backgroundFetchSize: 2,
+    fetchMethod: async () => {
+      fetchCalls++
+      return deferred.promise
+    },
+  })
+
+  c.set(1, 1)
+  clock.advance(100)
+  c.backgroundFetchSize = Symbol('2') as unknown as number
+  const refresh = c.fetch(1)
+
+  t.equal(fetchCalls, 1)
+  t.equal(c.size, 1)
+  t.equal(c.calculatedSize, 5)
+
+  deferred.resolve(2)
+  t.equal(await refresh, 2)
+  t.equal(c.size, 1)
+  t.equal(c.calculatedSize, 5)
+})
+
+t.test('backgroundFetchSize 0 retains in-flight coalescing', async t => {
+  const deferred = Promise.withResolvers<number>()
+  let fetchCalls = 0
+  const c = new LRUCache<number, number>({
+    maxSize: 10,
+    sizeCalculation: () => 5,
+    backgroundFetchSize: 0,
+    fetchMethod: async () => {
+      fetchCalls++
+      return deferred.promise
+    },
+  })
+
+  const first = c.fetch(1)
+  const second = c.fetch(1)
+  t.equal(fetchCalls, 1)
+  t.equal(c.size, 1)
+  t.equal(c.calculatedSize, 0)
+
+  deferred.resolve(1)
+  t.same(await Promise.all([first, second]), [1, 1])
+  t.equal(c.size, 1)
+  t.equal(c.calculatedSize, 5)
+})