Skip to content

Commit dd5b06a

Browse files
committed
even more iteration protection
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
1 parent 6406220 commit dd5b06a

File tree

2 files changed

+40
-4
lines changed

2 files changed

+40
-4
lines changed

Diff for: index.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -244,12 +244,14 @@ class LRUCache {
244244
*indexes ({ allowStale = this.allowStale } = {}) {
245245
if (this.size) {
246246
for (let i = this.tail, j; true; ) {
247+
if (!this.isValidIndex(i)) {
248+
break
249+
}
247250
j = i === this.head
248251
if (allowStale || !this.isStale(i)) {
249252
yield i
250253
}
251-
// either head now, or WAS head and head was deleted
252-
if (i === this.head || j && !this.isValidIndex(i)) {
254+
if (i === this.head) {
253255
break
254256
} else {
255257
i = this.prev[i]
@@ -261,12 +263,14 @@ class LRUCache {
261263
*rindexes ({ allowStale = this.allowStale } = {}) {
262264
if (this.size) {
263265
for (let i = this.head, j; true; ) {
264-
j = i === this.tail
266+
if (!this.isValidIndex(i)) {
267+
break
268+
}
265269
if (allowStale || !this.isStale(i)) {
266270
yield i
267271
}
268272
// either the tail now, or WAS the tail, and deleted
269-
if (i === this.tail || j && !this.isValidIndex(i)) {
273+
if (i === this.tail) {
270274
break
271275
} else {
272276
i = this.next[i]

Diff for: test/delete-while-iterating.js

+32
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,35 @@ t.test('rdelete odds', t => {
6666
t.same([...c.keys()], [4, 2, 0])
6767
t.end()
6868
})
69+
70+
t.test('delete two of them', t => {
71+
const c = t.context
72+
t.same([...c.keys()], [4, 3, 2, 1, 0])
73+
for (const k of c.keys()) {
74+
if (k === 3) {
75+
c.delete(3)
76+
c.delete(4)
77+
} else if (k === 1) {
78+
c.delete(1)
79+
c.delete(0)
80+
}
81+
}
82+
t.same([...c.keys()], [2])
83+
t.end()
84+
})
85+
86+
t.test('rdelete two of them', t => {
87+
const c = t.context
88+
t.same([...c.keys()], [4, 3, 2, 1, 0])
89+
for (const k of c.rkeys()) {
90+
if (k === 3) {
91+
c.delete(3)
92+
c.delete(4)
93+
} else if (k === 1) {
94+
c.delete(1)
95+
c.delete(0)
96+
}
97+
}
98+
t.same([...c.keys()], [2])
99+
t.end()
100+
})

0 commit comments

Comments
 (0)