[BUG] ClassLoader Leak / Metaspace OOM: Unmanaged static ThreadLocal<Cipher> in KeystoreEncryptionSpi
- Dominant language
- Java
- Stars
- 5.1k
- Forks
- 1.9k
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
Description
Summary:
In `org.apache.ignite.spi.encryption.keystore.KeystoreEncryptionSpi`, a `static final ThreadLocal aesWithPadding` is used to cache Cipher instances for AES encryption. However, the codebase completely lacks a lifecycle management mechanism for this `ThreadLocal`—`aesWithPadding.remove()` is never called, even during the SPI stop phase (`spiStop()`).
Root Cause:
When a worker thread executes an encryption operation, a Cipher instance is placed into the thread's ThreadLocalMap. Because the key (`aesWithPadding`) is static final, it creates a permanent strong reference from the worker thread to the Cipher instance (and its associated JCA Provider classes).
Impact (Critical for Embedded Mode):
Apache Ignite is frequently deployed in embedded mode within Web Containers (e.g., Tomcat) or microservices.
If the host application's worker threads are used for these operations, the ThreadLocal attaches the Cipher to long-lived threads.
This creates a strong reference chain (Worker Thread -> ThreadLocalMap -> Cipher/Provider -> WebappClassLoader) that pins the application's classloader in memory.
Upon application undeployment or hot-redeployment, the old classloader cannot be garbage collected, inevitably leading to a java.lang.OutOfMemoryError: Metaspace.
Code Snippet
Location: KeystoreEncryptionSpi.java
// Definition: Static ThreadLocal without boundary management
```
private static final ThreadLocal aesWithPadding = ThreadLocal.withInitial(() -> {
try {
return Cipher.getInstance(AES_WITH_PADDING);
} catch (NoSuchAlgorithmException | NoSuchPaddingException e) {
throw new IgniteException(e);
}
});
```
(Note: `aesWithPadding.remove()` is never called in `doEncryption()` or `spiStop()`)
Expected Behavior
The SPI should ensure that thread-local resources are cleanly removed to prevent memory and classloader leaks.
Proposed Fix:
1. Targeted Cleanup: If the `ThreadLocal` must remain static for performance, expose a cleanup mechanism that iterates and clears the map, or ensure `aesWithPadding.remove()` is called in a try-finally block after the cipher operation is complete (if thread pooling allows).
2. Instance-level caching: Consider changing the static `ThreadLocal` to an instance-level `ThreadLocal` or a lightweight object pool bound to the `KeystoreEncryptionSpi` instance lifecycle, ensuring it can be cleared during `spiStop()`.
Contributor guide
Research direction
Start by reading KeystoreEncryptionSpi.java, especially the static aesWithPadding definition, doEncryption(), and spiStop(). Determine which lifecycle strategy fits the SPI and embedded or pooled-thread use, then verify that the selected approach removes the thread-local resource during the relevant operation or stop phase without breaking encryption behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100