From 0a2a863d53747a950fcdd706650e1bbb1dc4cfc7 Mon Sep 17 00:00:00 2001 From: Rajat Jain Date: Tue, 21 Jul 2026 14:34:54 +0530 Subject: [PATCH] Fix memory leak in CacheManager by reusing InheritableThreadLocal instance --- .../protocol/http/control/CacheManager.java | 21 +++-- .../control/TestCacheManagerMemoryLeak.java | 84 +++++++++++++++++++ .../TestCacheManagerThreadIteration.java | 15 ++++ 3 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerMemoryLeak.java diff --git a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java index 8852f6332e4..876012ee5c3 100644 --- a/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java +++ b/src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java @@ -602,15 +602,18 @@ public void clear(){ private void clearCache() { log.debug("Clear cache"); - // TODO: avoid re-creating the thread local every time, reset its contents instead - threadCache = new InheritableThreadLocal>(){ - @Override - protected Cache initialValue() { - return Caffeine.newBuilder() - .maximumSize(getMaxSize()) - .build(); - } - }; + if (threadCache == null) { + threadCache = new InheritableThreadLocal>(){ + @Override + protected Cache initialValue() { + return Caffeine.newBuilder() + .maximumSize(getMaxSize()) + .build(); + } + }; + } else { + threadCache.remove(); + } } /** diff --git a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerMemoryLeak.java b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerMemoryLeak.java new file mode 100644 index 00000000000..82d985729a9 --- /dev/null +++ b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerMemoryLeak.java @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.jmeter.protocol.http.control; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; + +import java.lang.reflect.Field; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +public class TestCacheManagerMemoryLeak { + + @Test + @Timeout(30) + public void testBenchmarkThreadLocalReuseUnderConcurrentLoad() throws Exception { + CacheManager cacheManager = new CacheManager(); + cacheManager.setClearEachIteration(true); + + Field threadLocalField = CacheManager.class.getDeclaredField("threadCache"); + threadLocalField.setAccessible(true); + + Object initialThreadLocal = threadLocalField.get(cacheManager); + assertNotNull(initialThreadLocal, "Initial threadCache ThreadLocal should not be null"); + + int threadCount = 50; + int iterationsPerThread = 200; + ExecutorService executor = Executors.newFixedThreadPool(threadCount); + CountDownLatch startLatch = new CountDownLatch(1); + CountDownLatch finishLatch = new CountDownLatch(threadCount); + AtomicInteger mismatchCount = new AtomicInteger(0); + + for (int i = 0; i < threadCount; i++) { + executor.submit(() -> { + try { + startLatch.await(); + for (int j = 0; j < iterationsPerThread; j++) { + cacheManager.clear(); + Object current = threadLocalField.get(cacheManager); + if (current != initialThreadLocal) { + mismatchCount.incrementAndGet(); + } + } + } catch (Exception e) { + mismatchCount.incrementAndGet(); + } finally { + finishLatch.countDown(); + } + }); + } + + long startNs = System.nanoTime(); + startLatch.countDown(); + finishLatch.await(); + long durationMs = (System.nanoTime() - startNs) / 1_000_000; + + executor.shutdown(); + + assertEquals(0, mismatchCount.get(), "ThreadLocal instance mismatch count must be 0 (no new instances created)"); + System.out.println("Benchmark completed: " + (threadCount * iterationsPerThread) + + " clearCache iterations across " + threadCount + " threads in " + durationMs + " ms."); + } +} diff --git a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerThreadIteration.java b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerThreadIteration.java index 349fe143153..29f8ee9fc50 100644 --- a/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerThreadIteration.java +++ b/src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerThreadIteration.java @@ -397,4 +397,19 @@ public void testCacheManagerWhenThreadIterationIsSameUser() throws Exception { assertTrue(this.cacheManager.inCache(url, headers), "After iteration, should find valid entry"); } + @Test + public void testClearCacheReusesThreadLocalInstance() throws Exception { + Field threadLocalField = CacheManager.class.getDeclaredField("threadCache"); + threadLocalField.setAccessible(true); + Object initialThreadLocal = threadLocalField.get(this.cacheManager); + assertNotNull(initialThreadLocal, "ThreadLocal instance should be initialized"); + + for (int i = 0; i < 100; i++) { + this.cacheManager.clear(); + Object currentThreadLocal = threadLocalField.get(this.cacheManager); + assertSame(initialThreadLocal, currentThreadLocal, + "ThreadLocal instance should be reused across clear calls (iteration " + i + ")"); + } + } + }