fix(core): ClientCacheMemory.findCells never stops

Author: Hanssen0

.changeset/cute-views-strive.md

@@ -0,0 +1,6 @@
+---
+"@ckb-ccc/core": patch
+---
+
+fix(core): `ClientCacheMemory.findCells` never stops
+  

packages/core/src/client/cache/memory.advanced.test.ts

@@ -0,0 +1,113 @@
+import { describe, expect, it } from "vitest";
+import { MapLru } from "./memory.advanced.js";
+
+describe("MapLru", () => {
+  it("should throw an error for invalid capacity", () => {
+    expect(() => new MapLru(0)).toThrow("Capacity must be a positive integer");
+    expect(() => new MapLru(-1)).toThrow("Capacity must be a positive integer");
+    expect(() => new MapLru(1.5)).toThrow(
+      "Capacity must be a positive integer",
+    );
+  });
+
+  it("should set and get values correctly", () => {
+    const cache = new MapLru<string, number>(2);
+    cache.set("a", 1);
+    cache.set("b", 2);
+
+    expect(cache.get("a")).toBe(1);
+    expect(cache.get("b")).toBe(2);
+    expect(cache.size).toBe(2);
+  });
+
+  it("should evict the least recently used item when capacity is exceeded", () => {
+    const cache = new MapLru<string, number>(2);
+    cache.set("a", 1);
+    cache.set("b", 2);
+    cache.set("c", 3); // This should evict "a"
+
+    expect(cache.has("a")).toBe(false);
+    expect(cache.get("b")).toBe(2);
+    expect(cache.get("c")).toBe(3);
+    expect(cache.size).toBe(2);
+  });
+
+  it("should update the recently used status on get, affecting eviction order", () => {
+    const cache = new MapLru<string, number>(2);
+    cache.set("a", 1);
+    cache.set("b", 2);
+    cache.get("a"); // "a" is now the most recently used
+    cache.set("c", 3); // This should evict "b"
+
+    expect(cache.has("b")).toBe(false);
+    expect(cache.get("a")).toBe(1);
+    expect(cache.get("c")).toBe(3);
+    expect(cache.size).toBe(2);
+  });
+
+  it("should handle deletion correctly", () => {
+    const cache = new MapLru<string, number>(2);
+    cache.set("a", 1);
+    cache.set("b", 2);
+    cache.delete("a");
+
+    expect(cache.has("a")).toBe(false);
+    expect(cache.get("b")).toBe(2);
+    expect(cache.size).toBe(1);
+
+    // @ts-expect-error - accessing private property for testing
+    expect(cache.lru.has("a")).toBe(false);
+  });
+
+  describe("iteration behavior", () => {
+    it("should handle LRU updates when an item is accessed during iteration", () => {
+      const cache = new MapLru<string, number>(3);
+      cache.set("a", 1).set("b", 2).set("c", 3);
+
+      // Initial LRU order: a, b, c (c is MRU)
+      // @ts-expect-error - accessing private property for testing
+      expect(Array.from(cache.lru.keys())).toEqual(["a", "b", "c"]);
+
+      for (const [key] of cache.entries()) {
+        if (key === "b") {
+          cache.get("a"); // Access 'a', making it the new MRU
+        }
+      }
+
+      // Final LRU order should be: b, c, a
+      // @ts-expect-error - accessing private property for testing
+      expect(Array.from(cache.lru.keys())).toEqual(["b", "c", "a"]);
+
+      // Adding a new item should evict the new LRU item, which is 'b'
+      cache.set("d", 4);
+      expect(cache.has("b")).toBe(false);
+      expect(cache.has("c")).toBe(true);
+      expect(cache.has("a")).toBe(true);
+      expect(cache.has("d")).toBe(true);
+    });
+
+    it("should handle modifications and evictions during iteration", () => {
+      const cache = new MapLru<string, number>(3);
+      cache.set("a", 1).set("b", 2).set("c", 3);
+
+      // Initial state: keys are [a, b, c], LRU order is [a, b, c]
+      const visited: string[] = [];
+      // The standard Map iterator will visit newly added items.
+      // When we add "d", "a" gets evicted. The iterator, having already passed "a",
+      // will continue to "c" and then visit the new item "d".
+      for (const [key] of cache.entries()) {
+        visited.push(key);
+        if (key === "b") {
+          cache.set("d", 4); // This will evict 'a'
+        }
+      }
+
+      expect(visited).toEqual(["a", "b", "c", "d"]);
+
+      // Final state of the cache
+      expect(cache.has("a")).toBe(false);
+      expect(cache.size).toBe(3);
+      expect(Array.from(cache.keys())).toEqual(["b", "c", "d"]);
+    });
+  });
+});

packages/core/src/client/cache/memory.advanced.ts

@@ -145,73 +145,80 @@ export function filterCell(
 }
 
 /**
- * A Least Recently Used (LRU) cache implemented using a Map.
- *
- * This class extends the built-in Map to provide an LRU cache with a fixed capacity.
- * When the cache is full, the least recently used entry is automatically evicted.
- *
- * @template K The type of the keys in the cache.
- * @template V The type of the values in the cache.
+ * A Map-like class that implements a "Least Recently Used" (LRU) cache policy.
+ * When the cache reaches its capacity, the least recently used item is removed.
+ * @public
  */
 export class MapLru<K, V> extends Map<K, V> {
+  private readonly lru: Set<K> = new Set<K>();
+
   /**
-   * Constructs a new MapLru instance.
-   *
-   * @param capacity The maximum number of entries the cache can hold. Must be a positive integer.
-   * @throws {Error} If the capacity is not a positive integer.
+   * @param capacity - The maximum number of items to store in the cache. Must be a positive integer.
    */
   constructor(private readonly capacity: number) {
     super();
+
     if (!Number.isInteger(capacity) || capacity < 1) {
       throw new Error("Capacity must be a positive integer");
     }
   }
 
   /**
-   * Retrieves a value from the cache.
-   *
-   * If the key is present in the cache, the value is moved to the most-recently-used position.
-   *
-   * @param key The key of the value to retrieve.
-   * @returns The value associated with the key, or undefined if the key is not present.
+   * Retrieves the value for a given key and marks it as recently used.
+   * @param key - The key of the element to retrieve.
+   * @returns The value associated with the key, or `undefined` if the key is not in the cache.
    */
-  override get(key: K): V | undefined {
-    // Check if the key exists. If not, return undefined.
+  override get(key: K) {
     if (!super.has(key)) {
-      return undefined;
+      return;
     }
 
-    const value = super.get(key) as V;
+    this.lru.delete(key);
+    this.lru.add(key);
 
-    // Move to most-recently-used position
-    super.delete(key);
-    super.set(key, value);
-
-    return value;
+    return super.get(key);
   }
 
   /**
-   * Inserts a new value into the cache, or updates an existing value.
-   *
-   * If the key is already present in the cache, it is first deleted so that the re-insertion
-   * moves it to the most-recently-used position.
-   * If the cache is over capacity after the insertion, the least recently used entry is evicted.
-   *
-   * @param key The key of the value to insert or update.
-   * @param value The value to associate with the key.
-   * @returns This MapLru instance.
+   * Adds or updates a key-value pair in the cache and marks the key as recently used.
+   * If setting a new key causes the cache to exceed its capacity, the least recently used item is evicted.
+   * @param key - The key of the element to add or update.
+   * @param value - The value of the element to add or update.
+   * @returns The `MapLru` instance.
    */
-  override set(key: K, value: V): this {
-    // Delete and re-insert to move key to the end (most-recently-used)
-    super.delete(key);
+  override set(key: K, value: V) {
     super.set(key, value);
 
-    // Evict oldest if over capacity
-    if (super.size > this.capacity) {
-      const oldestKey = super.keys().next().value!;
-      super.delete(oldestKey);
-    }
+    this.lru.delete(key);
+    this.lru.add(key);
 
+    // Evict the oldest entry if capacity is exceeded.
+    if (this.lru.size > this.capacity) {
+      const oldestKey = this.lru.keys().next().value!;
+      this.delete(oldestKey);
+    }
     return this;
   }
+
+  /**
+   * Removes the specified element from the cache.
+   * @param key - The key of the element to remove.
+   * @returns `true` if an element in the `MapLru` object existed and has been removed, or `false` if the element does not exist.
+   */
+  override delete(key: K): boolean {
+    if (!super.delete(key)) {
+      return false;
+    }
+
+    this.lru.delete(key);
+    return true;
+  }
+
+  /**
+   * Removes all key-value pairs from the cache.
+   */
+  override clear() {
+    super.clear();
+    this.lru.clear();
+  }
 }