even more iteration protection

isaacs · isaacs · commit dd5b06a611ed · 2022-03-14T10:41:08.000-07:00
There's really no way around checking if the iteration index is valid. This makes me a little bit sad, because testing for index validity is slow af. But I suppose cache.entries() and friends are not really likely to be used in performance-critical paths as much as set() and get(). Re: #212
diff --git a/index.js b/index.js
@@ -244,12 +244,14 @@ class LRUCache {
   *indexes ({ allowStale = this.allowStale } = {}) {
     if (this.size) {
       for (let i = this.tail, j; true; ) {
+        if (!this.isValidIndex(i)) {
+          break
+        }
         j = i === this.head
         if (allowStale || !this.isStale(i)) {
           yield i
         }
-        // either head now, or WAS head and head was deleted
-        if (i === this.head || j && !this.isValidIndex(i)) {
+        if (i === this.head) {
           break
         } else {
           i = this.prev[i]
@@ -261,12 +263,14 @@ class LRUCache {
   *rindexes ({ allowStale = this.allowStale } = {}) {
     if (this.size) {
       for (let i = this.head, j; true; ) {
-        j = i === this.tail
+        if (!this.isValidIndex(i)) {
+          break
+        }
         if (allowStale || !this.isStale(i)) {
           yield i
         }
         // either the tail now, or WAS the tail, and deleted
-        if (i === this.tail || j && !this.isValidIndex(i)) {
+        if (i === this.tail) {
           break
         } else {
           i = this.next[i]
diff --git a/test/delete-while-iterating.js b/test/delete-while-iterating.js
@@ -66,3 +66,35 @@ t.test('rdelete odds', t => {
   t.same([...c.keys()], [4, 2, 0])
   t.end()
 })
+
+t.test('delete two of them', t => {
+  const c = t.context
+  t.same([...c.keys()], [4, 3, 2, 1, 0])
+  for (const k of c.keys()) {
+    if (k === 3) {
+      c.delete(3)
+      c.delete(4)
+    } else if (k === 1) {
+      c.delete(1)
+      c.delete(0)
+    }
+  }
+  t.same([...c.keys()], [2])
+  t.end()
+})
+
+t.test('rdelete two of them', t => {
+  const c = t.context
+  t.same([...c.keys()], [4, 3, 2, 1, 0])
+  for (const k of c.rkeys()) {
+    if (k === 3) {
+      c.delete(3)
+      c.delete(4)
+    } else if (k === 1) {
+      c.delete(1)
+      c.delete(0)
+    }
+  }
+  t.same([...c.keys()], [2])
+  t.end()
+})
