-
Notifications
You must be signed in to change notification settings - Fork 120
Description
Description
I identified a potential ThreadLocal leak in POJOActivityTaskHandler.java within the POJOLocalActivityImplementation inner class.
The CurrentActivityExecutionContext is set before entering the try-finally block intended to clean it up. Specifically, task.getInput() is called between set() and the try block. If task.getInput() throws an unchecked exception (e.g., OutOfMemoryError, NullPointerException, or internal protocol error), the method exits immediately, and unset() is never called.
Root Cause
In POJOActivityTaskHandler.java:
// Inner class: POJOLocalActivityImplementation
@OverRide
public ActivityTaskHandler.Result execute(ActivityTask task, Scope metricsScope) {
ActivityExecutionContext context =
new LocalActivityExecutionContextImpl(service, domain, task);
// [RISK] 1. ThreadLocal is set here
CurrentActivityExecutionContext.set(context);
// [RISK] 2. If getInput() throws an exception, the method exits here.
// The finally block is bypassed.
byte[] input = task.getInput();
try {
// ... execution logic ...
} finally {
// 3. Cleanup is skipped if step 2 fails
CurrentActivityExecutionContext.unset();
}
}
Impact
If this occurs, the worker thread remains polluted with a stale ActivityExecutionContext.
-
Crash: Subsequent reuse of this thread for a new activity will fail with java.lang.IllegalStateException: current already set.
-
Crosstalk: There is a risk of context data crosstalk between workflow executions if the check mechanism fails.
Suggested Fix
Move CurrentActivityExecutionContext.set(context) inside the try block, or move task.getInput() before the set() call (consistent with how the standard POJOActivityImplementation correctly handles this).