apache / apache/jmeter

Memory Leak in `CacheManager`: `InheritableThreadLocal` Recreated Without Cleanup on Every Iteration

Open
#6,730 0 comments 0 reactions 0 assignees View on GitHub
defect to-triage
Dominant language
Java
Stars
9.5k
Forks
2.3k
Avg merge
1d 22h
Merged PRs (30d)
5

Description

## 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> 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>() { // NEW instance every call
@Override
protected Cache 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` 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` 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>() {
@Override
protected Cache 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>() {
@Override
protected Cache initialValue() {
return Caffeine.newBuilder()
.maximumSize(getMaxSize())
.build();
}
};
} else {
// Invalidate all entries in the existing Cache for this thread — no new objects
Cache 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 |

---

## 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.

### 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)

Contributor guide

Open the contributing guide

Research direction

Start with clearCache() and testIterationStart() in src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java, then read TestCacheManagerThreadIteration. Verify the existing threadCache reflection tests and extend coverage for repeated cache clearing; done means the thread-local is reused without retaining prior per-thread caches and the relevant tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
performance, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.