Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: isaacs/node-lru-cache
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v7.2.2
Choose a base ref
...
head repository: isaacs/node-lru-cache
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v7.2.3
Choose a head ref
  • 2 commits
  • 6 files changed
  • 1 contributor

Commits on Apr 9, 2022

  1. Fix memory leak in unbounded storage case 

    Evictions were no being tracked properly when maxSize triggered an
eviction.

In the max-bound storage case, this wasn't so much of a problem, because
the storage couldn't grow unbounded anyway.  However, since size-evicted
indexes were not being added to the `free` list, it would effectively
reduce the available storage space, holding onto old entries and
shortening the available storage area, which would result in fewer items
being cached (except some that were cached indefinitely!)

In the unbounded case, this was much worse.  Because the storage array
was unbounded, the cache effectively always remained in the initial
"fill to completion" state, resulting in holding onto _every_ item
forever, while thinking that the `calculatedSize` was being limited to
`maxSize`!  Pretty bad.

(This will be back-ported to other 7.x versions as a patch.)

Thanks to @moish83 for the report.

Fix: #227
    @isaacs
    isaacs committed Apr 9, 2022
    Copy the full SHA
    5970500 View commit details

  2. 7.2.3

    @isaacs
    isaacs committed Apr 9, 2022
    Copy the full SHA
    8373257 View commit details
Showing with 904 additions and 1,481 deletions.
  1. +11 −4 index.js
  2. +704 −1,474 package-lock.json
  3. +7 −3 package.json
  4. +1 −0 scripts/.gitignore
  5. +68 −0 scripts/max-size-test.js
  6. +113 −0 test/avoid-memory-leak.js
15 changes: 11 additions & 4 deletions index.js
View file
Original file line number Diff line number Diff line change
@@ -111,6 +111,7 @@ class LRUCache {
throw new TypeError('sizeCalculating set to non-function')
}
}

this.keyMap = new Map()
this.keyList = new Array(max).fill(null)
this.valList = new Array(max).fill(null)
@@ -212,7 +213,7 @@ class LRUCache {
this.sizes[index] = isPosInt(s) ? s : 0
const maxSize = this.maxSize - this.sizes[index]
while (this.calculatedSize > maxSize) {
this.evict()
this.evict(true)
}
this.calculatedSize += this.sizes[index]
}
@@ -390,7 +391,7 @@ class LRUCache {
return this.tail
}
if (this.size === this.max) {
return this.evict()
return this.evict(false)
}
if (this.free.length !== 0) {
return this.free.pop()
@@ -402,17 +403,23 @@ class LRUCache {
pop () {
if (this.size) {
const val = this.valList[this.head]
this.evict()
this.evict(true)
return val
}
}

evict () {
evict (free) {
const head = this.head
const k = this.keyList[head]
const v = this.valList[head]
this.dispose(v, k, 'evict')
this.removeItemSize(head)
// if we aren't about to use the index, then null these out
if (free) {
this.keyList[head] = null
this.valList[head] = null
this.free.push(head)
}
this.head = this.next[head]
this.keyMap.delete(k)
this.size --
Loading