ThreadLocal pollution in HystrixTimer causing metric attribution errors due to unsafe thread reuse

I identified a ThreadLocal pollution issue in com.netflix.hystrix.util.HystrixTimer.

The HystrixTimer uses a ScheduledThreadPoolExecutor to reuse threads for timer tasks. However, it lacks a mechanism to clean up ThreadLocal context after task execution. Critically, the Javadoc for TimerListener.tick() explicitly encourages users to set ThreadLocals inside the method (Lines 190-192), but the run() implementation (Lines 96-106) does not ensure these are cleared, especially if an exception occurs or the user forgets the finally block.

Source Code Location: // HystrixTimer.java

// 1. Misleading Javadoc (Lines 190-192): // "If you need a ThreadLocal set, you can store the state in the TimerListener, // then when tick() is called, set the ThreadLocal to your desired value."

// 2. Unsafe Execution (Lines 96-106): Runnable r = new Runnable() { @Override public void run() { try { listener.tick(); } catch (Exception e) { logger.error("Failed while ticking TimerListener", e); } // MISSING: Finally block to clear/reset ThreadLocals or HystrixRequestContext } };

Impact

1. Metric Deviation: Latency and timeout metrics may be recorded under the wrong CommandKey or CommandGroup if the HystrixRequestContext leaks from a previous execution on the same thread.

2. Context Pollution: Downstream logic executed by the timer thread may inherit stale user contexts (e.g., User ID, Trace ID).

Expected behavior The HystrixTimer should either:

1. Sanitize the thread state before/after execution.

2. Or, at minimum, the Javadoc should strictly warn users that try-finally cleanup is mandatory because threads are reused.

Actual behavior Threads in the HystrixTimer pool retain ThreadLocal values from previous tick() executions, leading to "cross-talk" between different Hystrix commands.

Suggested Fix Update the Runnable wrapper to ensure cleanup, or update documentation to warn about thread pooling risks.

Code Fix Example: Runnable r = new Runnable() { @Override public void run() { try { listener.tick(); } catch (Exception e) { logger.error(...); } finally { // Ideally: HystrixRequestContext.getContextForCurrentThread().shutdown(); // Or explicitly warn users to clean up. } } };