From 39536acf9f28ddcae59bf89ae2c31b8e6a615494 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 9 Apr 2020 15:38:01 +0200 Subject: [PATCH] Fix potential leak in MimeTypeUtils LRUCache Prior to this commit, the `MimeTypeUtils` LRUCache would maintain a queue to track least recently used cached values. In some cases, concurrent access could create more entries in that unbounded queue than expected and spend a significant amount of time removing entries in that queue (i.e. iterating over a long list of elements). This commit ensures that recently used entries are only added to the queue if they've been removed by the current thread, in case of concurrent access. Fixes gh-24886 --- .../org/springframework/util/MimeTypeUtils.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java b/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java index c369bb4ebc4..490437d6e2b 100644 --- a/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java +++ b/spring-core/src/main/java/org/springframework/util/MimeTypeUtils.java @@ -29,7 +29,7 @@ import java.util.List; import java.util.Map; import java.util.Random; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Function; @@ -420,7 +420,7 @@ public abstract class MimeTypeUtils { private final int maxSize; - private final ConcurrentLinkedQueue queue = new ConcurrentLinkedQueue<>(); + private final ConcurrentLinkedDeque queue = new ConcurrentLinkedDeque<>(); private final ConcurrentHashMap cache = new ConcurrentHashMap<>(); @@ -446,8 +446,9 @@ public abstract class MimeTypeUtils { } this.lock.readLock().lock(); try { - this.queue.remove(key); - this.queue.add(key); + if (this.queue.removeLastOccurrence(key)) { + this.queue.offer(key); + } return cached; } finally { @@ -459,8 +460,9 @@ public abstract class MimeTypeUtils { // Retrying in case of concurrent reads on the same key cached = this.cache.get(key); if (cached != null) { - this.queue.remove(key); - this.queue.add(key); + if (this.queue.removeLastOccurrence(key)) { + this.queue.offer(key); + } return cached; } // Generate value first, to prevent size inconsistency @@ -473,7 +475,7 @@ public abstract class MimeTypeUtils { cacheSize--; } } - this.queue.add(key); + this.queue.offer(key); this.cache.put(key, value); this.size = cacheSize + 1; return value;