-
Notifications
You must be signed in to change notification settings - Fork 324
Description
Bug Description
The BlockingAdaptiveExecutor defaults to using Executors.newCachedThreadPool() when no executor is provided. While the class relies on the Limiter to control concurrency, the underlying CachedThreadPool has no upper bound on thread creation (Integer.MAX_VALUE).
If the Limiter is misconfigured (e.g., high limits) or fails to throttle requests correctly during a traffic burst, the CachedThreadPool will spawn a new native thread for every request, bypassing the OS ulimit and causing java.lang.OutOfMemoryError: unable to create new native thread.
Code Analysis
In src/main/java/com/netflix/concurrency/limits/executors/BlockingAdaptiveExecutor.java:
- Builder Default (Line 79):
if (executor == null) {
// Defaults to unbounded CachedThreadPool
executor = Executors.newCachedThreadPool(new ThreadFactory() { ... });
} - Deprecated Constructor (Line 113):
public BlockingAdaptiveExecutor(Limiter limiter) {
this(limiter, Executors.newCachedThreadPool());
}
Impact
This design creates a "soft" dependency on the Limiter for system stability. A robust system should implement "defense in depth" by enforcing a hard limit at the thread pool level (Bounded Queue/Max Threads) to prevent JVM crashes even if the Limiter logic permits a surge.
Suggested Fix
Replace the default CachedThreadPool with a bounded ThreadPoolExecutor (e.g., with a reasonable max thread count and a blocking/rejecting policy), or require the user to explicitly provide an Executor.