Skip to content

Commit

Permalink
Fix stuck in LruCache after updating items (#3365)
Browse files Browse the repository at this point in the history
Inside the LruCache, it always create a new link node when doing the []= operator, what if we're doing the "updating" operation?

The _entries will replace the new value to the same key entry, but the link list will have two nodes with a same key, once the capacity is exhausted, the application will stuck in removing item as the second removing with the same key changes nothing.

You can reproduce this case by executing the test cases provided by this PR.
  • Loading branch information
linroid committed Sep 9, 2022
1 parent 9d607d5 commit 62cef9f
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 0 deletions.
1 change: 1 addition & 0 deletions build_runner_core/CHANGELOG.md
@@ -1,6 +1,7 @@
## 7.2.4-dev

- Fix some new lints.
- Fix a bug in the LRU cache.

## 7.2.3

Expand Down
1 change: 1 addition & 0 deletions build_runner_core/lib/src/asset/lru_cache.dart
Expand Up @@ -27,6 +27,7 @@ class LruCache<K, V> {
}

void operator []=(K key, V value) {
remove(key);
var entry = _Link(key, value, _computeWeight(value));
// Don't cache at all if above the individual weight max.
if (entry.weight > _individualWeightMax) {
Expand Down
53 changes: 53 additions & 0 deletions build_runner_core/test/asset/lru_cache_test.dart
@@ -1,6 +1,7 @@
// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
@Timeout(Duration(seconds: 10))

import 'package:build_runner_core/src/asset/lru_cache.dart';
import 'package:test/test.dart';
Expand Down Expand Up @@ -66,4 +67,56 @@ void main() {
expect(cache['3'], null);
expect(cache['4'], null);
});

test('can update values', () {
cache['a'] = 1;
cache['b'] = 2;
expect(cache['a'], 1);
expect(cache['b'], 2);

cache['a'] = 3;
cache['b'] = 4;
expect(cache['a'], 3);
expect(cache['b'], 4);
});

test('removes least recently used or updated items when full', () {
// Populate the cache until full.
var total = 0;
var n = 0;
while (total + maxIndividualWeight <= maxTotalWeight) {
total += maxIndividualWeight;
cache['$n'] = maxIndividualWeight;
n++;
}
// Read all the items in order, the first is now the least recently used.
for (var i = 0; i < n; i++) {
expect(cache['$i'], maxIndividualWeight);
}

// Update the first item, then the second is now the least recently used.
cache['0'] = maxIndividualWeight;
// Add another item, the second item should now be removed.
cache['${n + 1}'] = maxIndividualWeight;
expect(cache['1'], null);

// Read the first n-1 items, then the latest added item is now the least
// recently used.
for (var i = 0; i < n; i++) {
if (i != 1) {
expect(cache['$i'], maxIndividualWeight);
}
}

// Update all the items in order, the last added item should be removed
for (var i = 0; i < n; i++) {
cache['$i'] = maxIndividualWeight;
}
expect(cache['$n'], null);

// Check the first n entries shouldn't be removed
for (var i = 0; i < n; i++) {
expect(cache['$i'], maxIndividualWeight);
}
});
}

0 comments on commit 62cef9f

Please sign in to comment.