[I] Memory Leak in `CacheManager`: `InheritableThreadLocal` Recreated Without Cleanup on Every Iteration [jmeter]

["rajat315315 (via GitHub)" <gi...@apache.org>]

rajat315315 opened a new issue, #6730:
URL: https://github.com/apache/jmeter/issues/6730

   ### Expected behavior
   
   ## Summary
   
   `CacheManager.clearCache()` replaces the `threadCache` field with a **brand-new `InheritableThreadLocal` instance** on every call, but never calls `.remove()` on the previous instance first. Because `clearCache()` is invoked on every loop iteration (when "Clear cache each iteration" is enabled), this causes a **progressive memory leak** that grows with iteration count × thread count.
   
   The code itself acknowledges the problem with an existing TODO comment that has never been resolved:
   
   ```java
   // TODO: avoid re-creating the thread local every time, reset its contents instead
   ```
   
   ---
   
   ## Affected File
   
   `src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java`
   
   ---
   
   ## Root Cause Analysis
   
   ### The field
   
   ```java
   // CacheManager.java, line 83
   private transient InheritableThreadLocal<Cache<String, CacheEntry>> threadCache;
   ```
   
   ### The leak trigger — called every iteration
   
   ```java
   // CacheManager.java, lines 642-650
   @Override
   public void testIterationStart(LoopIterationEvent event) {
       JMeterVariables jMeterVariables = JMeterContextService.getContext().getVariables();
       if ((getControlledByThread() && !jMeterVariables.isSameUserOnNextIteration())
               || (!getControlledByThread() && getClearEachIteration())) {
           clearCache();   // <-- called on every qualifying iteration
       }
       useExpires = getUseExpires();
   }
   ```
   
   ### The buggy implementation
   
   ```java
   // CacheManager.java, lines 603-614
   private void clearCache() {
       log.debug("Clear cache");
       // TODO: avoid re-creating the thread local every time, reset its contents instead
       threadCache = new InheritableThreadLocal<Cache<String, CacheEntry>>() {  // NEW instance every call
           @Override
           protected Cache<String, CacheEntry> initialValue() {
               return Caffeine.newBuilder()
                       .maximumSize(getMaxSize())
                       .build();
           }
       };
       // MISSING: old threadCache.remove() is never called
   }
   ```
   
   ### Why this leaks
   
   Java's `ThreadLocal` (and `InheritableThreadLocal`) stores its values in a **`ThreadLocalMap`** inside each `Thread` object. The map is keyed by the `ThreadLocal` instance itself (via a weak reference to the key).
   
   When `clearCache()` replaces `threadCache` with a new object:
   
   1. The **field** now points to the new `InheritableThreadLocal`.
   2. The **old** `InheritableThreadLocal` instance is no longer reachable via the field — but its `ThreadLocalMap` **entries still exist** in every live JMeter worker thread.
   3. Because the `ThreadLocalMap` entries use **weak keys**, the entries *should* eventually be expunged — but **only when the stale key's weak reference is collected AND a subsequent `ThreadLocal` operation on that thread triggers cleanup**. In practice, with hundreds of live threads executing fast loops, this cleanup is severely delayed or never triggered during the test run.
   4. Each orphaned `ThreadLocalMap` entry holds a **strong reference** to the `Cache<String, CacheEntry>` value — a Caffeine cache that itself holds `CacheEntry` objects (URL strings, ETags, Last-Modified dates, Expires dates).
   5. With N threads and K iterations, up to **N × K** leaked `Cache` instances can accumulate.
   
   ### Memory impact
   
   Each leaked `Cache<String, CacheEntry>` includes:
   - The Caffeine internal data structures (capped at `maximumSize`, which defaults to **5000 entries**).
   - Retained `CacheEntry` objects: each contains `lastModified` (String), `etag` (String), `expires` (Date), and `varyHeader` (String).
   - For a test with 100 threads × 10,000 iterations: up to **1,000,000** orphaned `Cache` objects.
   
   ---
   
   ## Proposed Fix
   
   Instead of creating a new `InheritableThreadLocal` on every call, **reuse the same instance** and reset its contents by calling `.remove()` — which discards the cached value for the current thread only, causing `initialValue()` to re-run lazily on next access.
   
   ### Option A — Preferred: Reset via `.remove()` (reuse the same `ThreadLocal`)
   
   ```java
   private void clearCache() {
       log.debug("Clear cache");
       if (threadCache == null) {
           // First-time initialization only
           threadCache = new InheritableThreadLocal<Cache<String, CacheEntry>>() {
               @Override
               protected Cache<String, CacheEntry> initialValue() {
                   return Caffeine.newBuilder()
                           .maximumSize(getMaxSize())
                           .build();
               }
           };
       } else {
           // Reuse same ThreadLocal instance; evict only this thread's value.
           // Next call to getCache() will trigger initialValue() for a fresh Cache.
           threadCache.remove();
       }
   }
   ```
   
   This approach:
   - Creates exactly **one** `InheritableThreadLocal` per `CacheManager` instance (at construction time).
   - On each "clear" call, simply removes the current thread's value — no new objects created.
   - Does **not** leak any `ThreadLocalMap` entries.
   
   ### Option B — Alternative: Invalidate the existing Cache instead of replacing the ThreadLocal
   
   ```java
   private void clearCache() {
       log.debug("Clear cache");
       if (threadCache == null) {
           threadCache = new InheritableThreadLocal<Cache<String, CacheEntry>>() {
               @Override
               protected Cache<String, CacheEntry> initialValue() {
                   return Caffeine.newBuilder()
                           .maximumSize(getMaxSize())
                           .build();
               }
           };
       } else {
           // Invalidate all entries in the existing Cache for this thread — no new objects
           Cache<String, CacheEntry> existing = threadCache.get();
           if (existing != null) {
               existing.invalidateAll();
           }
       }
   }
   ```
   
   ---
   
   ## Impact
   
   | Configuration | Leak Frequency | Severity |
   |---|---|---|
   | "Clear cache each iteration" = ON | Every loop iteration per thread | 🔴 High |
   | `controlledByThread` = ON (new VU per iteration) | Every new-user iteration boundary | 🔴 High |
   | `clearEachIteration` = OFF | No leak from this path | Not affected |
   
   ---
   
   ## Environment
   
   - **Affected since:** JMeter 3.0 (when `InheritableThreadLocal` was introduced for `createCacheManagerProxy()` support). The TODO comment acknowledging this has been present since at least JMeter 3.1.
   - **Branch:** `main`
   - **Java versions:** 11, 17, 21 (same behavior across all LTS releases)
   - **JMeter versions:** 5.x and current `main`
   
   ---
   
   ## Related
   
   - The existing TODO comment at `CacheManager.java:605` directly calls out the intended fix:
     ```
     // TODO: avoid re-creating the thread local every time, reset its contents instead
     ```
   - `TestCacheManagerThreadIteration` uses reflection to inspect `threadCache` — any fix should ensure these tests continue to pass and ideally be extended to cover the leak scenario.
   
   
   ### Actual behavior
   
   _No response_
   
   ### Steps to reproduce the problem
   
   1. Create a Thread Group with **100+ threads** and **1000+ iterations**.
   2. Add an **HTTP Request** sampler targeting any server.
   3. Add a **HTTP Cache Manager** with the option **"Clear cache each iteration"** enabled (or with `controlledByThread` set such that the user changes on each iteration).
   4. Run the test while monitoring heap usage (e.g., via VisualVM, JFR, or `-verbose:gc`).
   5. Observe that heap usage climbs steadily throughout the test run and does **not** drop between iterations.
   
   
   ### JMeter Version
   
   6.0.0 (also affects 5.x releases)
   
   ### Java Version
   
   OpenJDK 25.0.3 (build 25.0.3+9-2-26.04.2-Ubuntu) / reproducible on Java 11, 17, 21+
   
   ### OS Version
   
   Ubuntu 26.04 LTS (Linux x86_64)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@jmeter.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org