Description
I found that LRUMap.set() in packages/core/src/utils/lru.ts:27 evicts the oldest entry whenever the map is full, even when the key being set already exists. Updating an existing key adds no new entries, so no eviction should happen, but the current code unconditionally drops this._cache.keys().next().value first. In the worst case this silently deletes live data: with maxSize=2 holding {a, b}, calling set('b', 22) deletes a and leaves the map with only {b} at size 1. It also never refreshes recency on update, because Map.set() on an existing key keeps its original insertion position. This affects every LRUMap consumer that rewrites keys (trace-propagation decision maps, span registries, file-content caches).
Reproduce
- Read
packages/core/src/utils/lru.ts:27-35:
public set(key: K, value: V): void {
if (this._cache.size >= this._maxSize) {
const nextKey = this._cache.keys().next().value!;
this._cache.delete(nextKey);
}
this._cache.set(key, value);
}
- Save this snippet as
/tmp/repro-lru.mjs (exact copy of the logic above) and run node /tmp/repro-lru.mjs:
const map = new LRUMap(2);
map.set('a', 1);
map.set('b', 2);
map.set('b', 22); // update existing key while full
console.log(map.keys(), map.size);
- Observed result:
['b'] with size === 1 -- entry 'a' was wrongly evicted. Expected ['a', 'b'] with size === 2.
Expected
set() on an already-present key should just update the value (and refresh its recency by delete-then-set) without evicting anything. Only inserting a genuinely new key into a full map should evict the oldest entry, e.g. guard the eviction with if (!this._cache.has(key) && this._cache.size >= this._maxSize) and delete the key first when it exists.
Checklist
Description
I found that
LRUMap.set()inpackages/core/src/utils/lru.ts:27evicts the oldest entry whenever the map is full, even when the key being set already exists. Updating an existing key adds no new entries, so no eviction should happen, but the current code unconditionally dropsthis._cache.keys().next().valuefirst. In the worst case this silently deletes live data: withmaxSize=2holding{a, b}, callingset('b', 22)deletesaand leaves the map with only{b}at size 1. It also never refreshes recency on update, becauseMap.set()on an existing key keeps its original insertion position. This affects everyLRUMapconsumer that rewrites keys (trace-propagation decision maps, span registries, file-content caches).Reproduce
packages/core/src/utils/lru.ts:27-35:/tmp/repro-lru.mjs(exact copy of the logic above) and runnode /tmp/repro-lru.mjs:['b']withsize === 1-- entry'a'was wrongly evicted. Expected['a', 'b']withsize === 2.Expected
set()on an already-present key should just update the value (and refresh its recency by delete-then-set) without evicting anything. Only inserting a genuinely new key into a full map should evict the oldest entry, e.g. guard the eviction withif (!this._cache.has(key) && this._cache.size >= this._maxSize)and delete the key first when it exists.Checklist