Merge pull request Netflix#1570 from mattrjacobs/only-wrap-timeout-ca…

…llable-when-necessary Only create the HystrixContextRunnable for running the timeout fallback...
rwilliams-r7 · May 8, 2017 · fefdc37 · fefdc37
2 parents f0d5482 + 951c6f9
commit fefdc37
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 12 deletions.
diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java
@@ -1132,17 +1132,8 @@ public Subscriber<? super R> call(final Subscriber<? super R> child) {
             // if the child unsubscribes we unsubscribe our parent as well
             child.add(s);
 
-            /*
-             * Define the action to perform on timeout outside of the TimerListener to it can capture the HystrixRequestContext
-             * of the calling thread which doesn't exist on the Timer thread.
-             */
-            final HystrixContextRunnable timeoutRunnable = new HystrixContextRunnable(originalCommand.concurrencyStrategy, new Runnable() {
-
-                @Override
-                public void run() {
-                    child.onError(new HystrixTimeoutException());
-                }
-            });
+            //capture the HystrixRequestContext upfront so that we can use it in the timeout thread later
+            final HystrixRequestContext hystrixRequestContext = HystrixRequestContext.getContextForCurrentThread();
 
             TimerListener listener = new TimerListener() {
 
@@ -1157,6 +1148,15 @@ public void tick() {
                         // shut down the original request
                         s.unsubscribe();
 
+                        final HystrixContextRunnable timeoutRunnable = new HystrixContextRunnable(originalCommand.concurrencyStrategy, hystrixRequestContext, new Runnable() {
+
+                            @Override
+                            public void run() {
+                                child.onError(new HystrixTimeoutException());
+                            }
+                        });
+
+
                         timeoutRunnable.run();
                         //if it did not start, then we need to mark a command start for concurrency metrics, and then issue the timeout
                     }

diff --git a/...x-core/src/main/java/com/netflix/hystrix/strategy/concurrency/HystrixContextRunnable.java b/...x-core/src/main/java/com/netflix/hystrix/strategy/concurrency/HystrixContextRunnable.java
@@ -34,6 +34,10 @@ public HystrixContextRunnable(Runnable actual) {
     }
 
     public HystrixContextRunnable(HystrixConcurrencyStrategy concurrencyStrategy, final Runnable actual) {
+        this(concurrencyStrategy, HystrixRequestContext.getContextForCurrentThread(), actual);
+    }
+
+    public HystrixContextRunnable(final HystrixConcurrencyStrategy concurrencyStrategy, final HystrixRequestContext hystrixRequestContext, final Runnable actual) {
         this.actual = concurrencyStrategy.wrapCallable(new Callable<Void>() {
 
             @Override
@@ -43,7 +47,7 @@ public Void call() throws Exception {
             }
 
         });
-        this.parentThreadState = HystrixRequestContext.getContextForCurrentThread();
+        this.parentThreadState = hystrixRequestContext;
     }
 
     @Override