-
Notifications
You must be signed in to change notification settings - Fork 50
API Design Risk: JBossExecutors facilitates silent task loss via exposed DiscardPolicy and discardingExecutor #284
Description
I have analyzed the source code of JBossExecutors.java and identified a potential API design risk regarding the exposure of unsafe RejectedExecutionHandler policies.
The utility class JBossExecutors provides convenient static factory methods to access DiscardPolicy, CallerRunsPolicy, and even a fully completely silent discardingExecutor.
Location: src/main/java/org/jboss/threads/JBossExecutors.java
Problematic APIs:
-
discardingExecutor() (Lines 84-96): Returns an executor that silently swallows all tasks.
-
discardPolicy() (Lines 155-158): Returns a raw DiscardPolicy.
-
callerRunsPolicy() (Lines 138-141): Returns a raw CallerRunsPolicy.
Risk Analysis (DPSE - Deny Policy Setting Error) While these policies have valid niche use cases, exposing them as first-class citizens in a general-purpose utility class increases the likelihood of misuse by developers.
Silent Failures: Developers might use discardPolicy() or discardingExecutor() for convenience, not fully realizing that in production environments (e.g., high load), critical tasks (audit logs, business events) will be dropped without any logging or exceptions. This makes debugging "missing data" issues nearly impossible.
Thread Blocking: Exposing callerRunsPolicy() encourages its use, which is known to cause severe performance degradation or deadlocks if applied to I/O threads (e.g., within XNIO/Netty workers often used with JBoss software).
Suggested Improvement To align with "secure by default" principles, I suggest the following changes:
- Documentation: Update the JavaDoc for these methods to explicitly warn about the risks (e.g., "WARNING: Tasks rejected by this policy are discarded silently. Do not use for critical business logic.").
- Deprecation: Consider marking discardingExecutor() and discardPolicy() as @deprecated to discourage their use in favor of policies that at least log a warning (e.g., loggingExceptionHandler).
// Current implementation allows easy access to silent failure:
public static Executor discardingExecutor() {
return DiscardingExecutor.INSTANCE;
}
public static RejectedExecutionHandler discardPolicy() {
return DISCARD_POLICY;
}
Environment
Version: Latest Master
Component: jboss-threads