You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2018/08/30 08:11:16 UTC

[GitHub] wu-sheng closed pull request #1605: fix bug: memory leak caused by using unique instances of HystrixCon…

wu-sheng closed pull request #1605: fix bug: memory leak caused by using unique instances of HystrixCon…
URL: https://github.com/apache/incubator-skywalking/pull/1605
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/HystrixConcurrencyStrategyInterceptor.java b/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/HystrixConcurrencyStrategyInterceptor.java
index 8ccce2ffa..31d144789 100644
--- a/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/HystrixConcurrencyStrategyInterceptor.java
+++ b/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/HystrixConcurrencyStrategyInterceptor.java
@@ -20,12 +20,18 @@
 package org.apache.skywalking.apm.plugin.hystrix.v1;
 
 import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy;
-import java.lang.reflect.Method;
+
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
 
+import java.lang.reflect.Method;
+import java.util.concurrent.atomic.AtomicReference;
+
 public class HystrixConcurrencyStrategyInterceptor implements InstanceMethodsAroundInterceptor {
+
+    private final AtomicReference<SWHystrixConcurrencyStrategyWrapper> wrapper = new AtomicReference<SWHystrixConcurrencyStrategyWrapper>();
+
     @Override
     public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
         MethodInterceptResult result) throws Throwable {
@@ -35,7 +41,15 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
     @Override
     public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
         Object ret) throws Throwable {
-        return new SWHystrixConcurrencyStrategyWrapper((HystrixConcurrencyStrategy)ret);
+        if (wrapper.get() == null) {
+            Object impl = objInst.getSkyWalkingDynamicField();
+            if (impl == null) {
+                wrapper.compareAndSet(null, new SWHystrixConcurrencyStrategyWrapper((HystrixConcurrencyStrategy) ret));
+            } else {
+                wrapper.compareAndSet(null, (SWHystrixConcurrencyStrategyWrapper) impl);
+            }
+        }
+        return wrapper.get();
     }
 
     @Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
diff --git a/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/SWHystrixConcurrencyStrategyWrapper.java b/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/SWHystrixConcurrencyStrategyWrapper.java
index 102dc7c05..bcb620194 100644
--- a/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/SWHystrixConcurrencyStrategyWrapper.java
+++ b/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/SWHystrixConcurrencyStrategyWrapper.java
@@ -20,11 +20,13 @@
 package org.apache.skywalking.apm.plugin.hystrix.v1;
 
 import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy;
-import java.util.concurrent.Callable;
+
 import org.apache.skywalking.apm.agent.core.conf.RuntimeContextConfiguration;
 import org.apache.skywalking.apm.agent.core.context.ContextManager;
 import org.apache.skywalking.apm.agent.core.context.RuntimeContextSnapshot;
 
+import java.util.concurrent.Callable;
+
 public class SWHystrixConcurrencyStrategyWrapper extends HystrixConcurrencyStrategy {
 
     private final HystrixConcurrencyStrategy delegate;
@@ -36,7 +38,10 @@ public SWHystrixConcurrencyStrategyWrapper(
 
     @Override
     public <T> Callable<T> wrapCallable(Callable<T> callable) {
-        return new WrappedCallable<T>(ContextManager.getRuntimeContext().capture(), super.wrapCallable(callable));
+        Callable<T> delegateCallable = delegate != null
+                ? delegate.wrapCallable(callable)
+                : super.wrapCallable(callable);
+        return new WrappedCallable<T>(ContextManager.getRuntimeContext().capture(), delegateCallable);
     }
 
     static class WrappedCallable<T> implements Callable<T> {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services