From 4182698eb91e6e8e2bbf1a48047f2c4d895fac99 Mon Sep 17 00:00:00 2001
From: Oliver Drotbohm <odrotbohm@vmware.com>
Date: Tue, 23 Nov 2021 01:09:49 +0100
Subject: [PATCH] #1701 - Fix potential resource leak in DummyInvocationUtils.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We now constrain the cache of controller proxy instances to 256 elements using Spring's ConcurrentLruCache to avoid instances created via DummyInvocationUtils.methodOn(Class<?>, Object…). The parameters are part of the cache key and used to expand the type-level mappings. If those vary for each call and a request creates a lot of links (>100000) the memory consumption grows significantly, first and foremost indefinitely.

Using the ThreadLocal will still make sure that the cache is local to a current request, so the proxies can actually be reused as the method invocations used to record the mappings would interfere for concurrent requests otherwise.

Removed obsolete generic parameter on the CacheKey type.
---
 .../server/core/DummyInvocationUtils.java     | 31 ++++++++++---------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/main/java/org/springframework/hateoas/server/core/DummyInvocationUtils.java b/src/main/java/org/springframework/hateoas/server/core/DummyInvocationUtils.java
index 5db512f9..70035b4e 100644
--- a/src/main/java/org/springframework/hateoas/server/core/DummyInvocationUtils.java
+++ b/src/main/java/org/springframework/hateoas/server/core/DummyInvocationUtils.java
@@ -17,9 +17,7 @@ package org.springframework.hateoas.server.core;
 
 import java.lang.reflect.Method;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.Iterator;
-import java.util.Map;
 import java.util.Objects;
 
 import org.aopalliance.intercept.MethodInterceptor;
@@ -28,6 +26,7 @@ import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.aop.target.EmptyTargetSource;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
+import org.springframework.util.ConcurrentLruCache;
 import org.springframework.util.ReflectionUtils;
 
 /**
@@ -37,7 +36,14 @@ import org.springframework.util.ReflectionUtils;
  */
 public class DummyInvocationUtils {
 
-	private static final ThreadLocal<Map<CacheKey<?>, Object>> CACHE = ThreadLocal.withInitial(HashMap::new);
+	private static final ThreadLocal<ConcurrentLruCache<CacheKey, Object>> CACHE = ThreadLocal
+			.withInitial(() -> new ConcurrentLruCache<CacheKey, Object>(256,
+					it -> {
+
+						InvocationRecordingMethodInterceptor interceptor = new InvocationRecordingMethodInterceptor(it.type,
+								it.arguments);
+						return getProxyWithInterceptor(it.type, interceptor, it.type.getClassLoader());
+					}));
 
 	/**
 	 * Method interceptor that records the last method invocation and creates a proxy for the return value that exposes
@@ -127,12 +133,7 @@ public class DummyInvocationUtils {
 
 		Assert.notNull(type, "Given type must not be null!");
 
-		return (T) CACHE.get().computeIfAbsent(CacheKey.of(type, parameters), it -> {
-
-			InvocationRecordingMethodInterceptor interceptor = new InvocationRecordingMethodInterceptor(it.type,
-					it.arguments);
-			return getProxyWithInterceptor(it.type, interceptor, type.getClassLoader());
-		});
+		return (T) CACHE.get().get(CacheKey.of(type, parameters));
 	}
 
 	/**
@@ -202,19 +203,19 @@ public class DummyInvocationUtils {
 		return (T) factory.getProxy(classLoader);
 	}
 
-	private static final class CacheKey<T> {
+	private static final class CacheKey {
 
-		private final Class<T> type;
+		private final Class<?> type;
 		private final Object[] arguments;
 
-		private CacheKey(Class<T> type, Object[] arguments) {
+		private CacheKey(Class<?> type, Object[] arguments) {
 
 			this.type = type;
 			this.arguments = arguments;
 		}
 
-		public static <T> CacheKey<T> of(Class<T> type, Object[] arguments) {
-			return new CacheKey<T>(type, arguments);
+		public static CacheKey of(Class<?> type, Object[] arguments) {
+			return new CacheKey(type, arguments);
 		}
 
 		/*
@@ -232,7 +233,7 @@ public class DummyInvocationUtils {
 				return false;
 			}
 
-			CacheKey<?> cacheKey = (CacheKey<?>) o;
+			CacheKey cacheKey = (CacheKey) o;
 
 			return Objects.equals(this.type, cacheKey.type) //
 					&& Arrays.equals(this.arguments, cacheKey.arguments);