From a99733bf6f1a055c7c57135e53c551ee04bde51f Mon Sep 17 00:00:00 2001 From: chenpengfei Date: Thu, 30 Aug 2018 20:44:46 +0800 Subject: [PATCH] fix memory leak in HystrixPlugins (#1610) * fix memory leak in HystrixPlugins --- ...HystrixConcurrencyStrategyInterceptor.java | 17 ++++++- .../hystrix/v1/HystrixPluginsInterceptor.java | 15 +++++- .../SWHystrixConcurrencyStrategyWrapper.java | 5 +- .../v1/SWHystrixPluginsWrapperCache.java | 46 +++++++++++++++++++ 4 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/SWHystrixPluginsWrapperCache.java 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 8ccce2ffab..5875b7665d 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 @@ -34,8 +34,21 @@ public class HystrixConcurrencyStrategyInterceptor implements InstanceMethodsAro @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, - Object ret) throws Throwable { - return new SWHystrixConcurrencyStrategyWrapper((HystrixConcurrencyStrategy)ret); + Object ret) throws Throwable { + SWHystrixPluginsWrapperCache wrapperCache = (SWHystrixPluginsWrapperCache) objInst.getSkyWalkingDynamicField(); + if (wrapperCache == null || wrapperCache.getSwHystrixConcurrencyStrategyWrapper() == null) { + synchronized (objInst) { + if (wrapperCache == null) { + wrapperCache = new SWHystrixPluginsWrapperCache(); + objInst.setSkyWalkingDynamicField(wrapperCache); + } + if (wrapperCache.getSwHystrixConcurrencyStrategyWrapper() == null) { + wrapperCache.setSwHystrixConcurrencyStrategyWrapper(new SWHystrixConcurrencyStrategyWrapper((HystrixConcurrencyStrategy) ret)); + } + } + } + + return wrapperCache.getSwHystrixConcurrencyStrategyWrapper(); } @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/HystrixPluginsInterceptor.java b/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/HystrixPluginsInterceptor.java index d29b62fd4a..bb25b41ff4 100644 --- a/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/HystrixPluginsInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/HystrixPluginsInterceptor.java @@ -40,7 +40,20 @@ public class HystrixPluginsInterceptor implements InstanceMethodsAroundIntercept @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Object ret) throws Throwable { - return new SWExecutionHookWrapper((HystrixCommandExecutionHook)ret); + SWHystrixPluginsWrapperCache wrapperCache = (SWHystrixPluginsWrapperCache) objInst.getSkyWalkingDynamicField(); + if (wrapperCache == null || wrapperCache.getSwExecutionHookWrapper() == null) { + synchronized (objInst) { + if (wrapperCache == null) { + wrapperCache = new SWHystrixPluginsWrapperCache(); + objInst.setSkyWalkingDynamicField(wrapperCache); + } + if (wrapperCache.getSwExecutionHookWrapper() == null) { + wrapperCache.setSwExecutionHookWrapper(new SWExecutionHookWrapper((HystrixCommandExecutionHook) ret)); + } + } + } + + return wrapperCache.getSwExecutionHookWrapper(); } @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 102dc7c058..55888c16eb 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 @@ -36,7 +36,10 @@ public class SWHystrixConcurrencyStrategyWrapper extends HystrixConcurrencyStrat @Override public Callable wrapCallable(Callable callable) { - return new WrappedCallable(ContextManager.getRuntimeContext().capture(), super.wrapCallable(callable)); + Callable delegateCallable = delegate != null + ? delegate.wrapCallable(callable) + : super.wrapCallable(callable); + return new WrappedCallable(ContextManager.getRuntimeContext().capture(), delegateCallable); } static class WrappedCallable implements Callable { diff --git a/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/SWHystrixPluginsWrapperCache.java b/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/SWHystrixPluginsWrapperCache.java new file mode 100644 index 0000000000..5b702934f9 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/hystrix-1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/hystrix/v1/SWHystrixPluginsWrapperCache.java @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.apm.plugin.hystrix.v1; + +/** + * {@link SWHystrixPluginsWrapperCache} record the {@link SWExecutionHookWrapper} and {@link SWHystrixConcurrencyStrategyWrapper} object for + * storing in EnhancedInstance#dynamicField together. + * + * @author chenpengfei + */ +public class SWHystrixPluginsWrapperCache { + private volatile SWExecutionHookWrapper swExecutionHookWrapper; + private volatile SWHystrixConcurrencyStrategyWrapper swHystrixConcurrencyStrategyWrapper; + + public SWExecutionHookWrapper getSwExecutionHookWrapper() { + return swExecutionHookWrapper; + } + + public void setSwExecutionHookWrapper(SWExecutionHookWrapper swExecutionHookWrapper) { + this.swExecutionHookWrapper = swExecutionHookWrapper; + } + + public SWHystrixConcurrencyStrategyWrapper getSwHystrixConcurrencyStrategyWrapper() { + return swHystrixConcurrencyStrategyWrapper; + } + + public void setSwHystrixConcurrencyStrategyWrapper(SWHystrixConcurrencyStrategyWrapper swHystrixConcurrencyStrategyWrapper) { + this.swHystrixConcurrencyStrategyWrapper = swHystrixConcurrencyStrategyWrapper; + } +} -- GitLab