Merge pull request #738 from YunaiV/master-5.0

Concurrency conflicts in Spring plugin #735

apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/EnhanceRequireObjectCache.java

@@ -18,25 +18,26 @@
 
 package org.apache.skywalking.apm.plugin.spring.mvc.commons;
 
-import java.lang.reflect.Method;
-import javax.servlet.http.HttpServletResponse;
 import org.springframework.web.context.request.NativeWebRequest;
 
+import javax.servlet.http.HttpServletResponse;
+import java.lang.reflect.Method;
+
 public class EnhanceRequireObjectCache {
     private PathMappingCache pathMappingCache;
-    private NativeWebRequest nativeWebRequest;
-    private HttpServletResponse httpResponse;
+    private ThreadLocal<NativeWebRequest> nativeWebRequest = new ThreadLocal<NativeWebRequest>();
+    private ThreadLocal<HttpServletResponse> httpResponse = new ThreadLocal<HttpServletResponse>();
 
     public void setPathMappingCache(PathMappingCache pathMappingCache) {
         this.pathMappingCache = pathMappingCache;
     }
 
     public HttpServletResponse getHttpServletResponse() {
-        return httpResponse == null ? (HttpServletResponse)nativeWebRequest.getNativeResponse() : httpResponse;
+        return httpResponse.get() == null ? (HttpServletResponse) nativeWebRequest.get().getNativeResponse() : httpResponse.get();
     }
 
     public void setNativeWebRequest(NativeWebRequest nativeWebRequest) {
-        this.nativeWebRequest = nativeWebRequest;
+        this.nativeWebRequest.set(nativeWebRequest);
     }
 
     public String findPathMapping(Method method) {
@@ -52,10 +53,12 @@ public class EnhanceRequireObjectCache {
     }
 
     public void setHttpResponse(HttpServletResponse httpResponse) {
-        this.httpResponse = httpResponse;
+        this.httpResponse.set(httpResponse);
     }
 
-    public HttpServletResponse getHttpResponse() {
-        return httpResponse;
+    public void clearRequestAndResponse() {
+        setNativeWebRequest(null);
+        setHttpResponse(null);
     }
+
 }

apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInteceptor.java → apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java

@@ -19,27 +19,28 @@
 
 package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor;
 
-import java.lang.reflect.Method;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
+import org.apache.skywalking.apm.agent.core.context.CarrierItem;
 import org.apache.skywalking.apm.agent.core.context.ContextCarrier;
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
 import org.apache.skywalking.apm.agent.core.context.tag.Tags;
 import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
 import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
-import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
-import org.apache.skywalking.apm.plugin.spring.mvc.commons.EnhanceRequireObjectCache;
-import org.apache.skywalking.apm.agent.core.context.CarrierItem;
-import org.apache.skywalking.apm.agent.core.context.ContextManager;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
+import org.apache.skywalking.apm.plugin.spring.mvc.commons.EnhanceRequireObjectCache;
 import org.springframework.web.context.request.RequestContextHolder;
 import org.springframework.web.context.request.ServletRequestAttributes;
 
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.lang.reflect.Method;
+
 /**
  * the abstract method inteceptor
  */
-public abstract class AbstractMethodInteceptor implements InstanceMethodsAroundInterceptor {
+public abstract class AbstractMethodInterceptor implements InstanceMethodsAroundInterceptor {
     public abstract String getRequestURL(Method method);
 
     @Override
@@ -72,15 +73,19 @@ public abstract class AbstractMethodInteceptor implements InstanceMethodsAroundI
     @Override
     public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
         Object ret) throws Throwable {
-        HttpServletResponse response = ((EnhanceRequireObjectCache)objInst.getSkyWalkingDynamicField()).getHttpServletResponse();
+        try {
+            HttpServletResponse response = ((EnhanceRequireObjectCache) objInst.getSkyWalkingDynamicField()).getHttpServletResponse();
 
-        AbstractSpan span = ContextManager.activeSpan();
-        if (response.getStatus() >= 400) {
-            span.errorOccurred();
-            Tags.STATUS_CODE.set(span, Integer.toString(response.getStatus()));
+            AbstractSpan span = ContextManager.activeSpan();
+            if (response.getStatus() >= 400) {
+                span.errorOccurred();
+                Tags.STATUS_CODE.set(span, Integer.toString(response.getStatus()));
+            }
+            ContextManager.stopSpan();
+            return ret;
+        } finally {
+            ((EnhanceRequireObjectCache)objInst.getSkyWalkingDynamicField()).clearRequestAndResponse();
         }
-        ContextManager.stopSpan();
-        return ret;
     }
 
     @Override

apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/InvokeHandlerMethodInterceptor.java

@@ -18,13 +18,14 @@
 
 package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor;
 
-import java.lang.reflect.Method;
-import javax.servlet.http.HttpServletResponse;
 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 org.apache.skywalking.apm.plugin.spring.mvc.commons.EnhanceRequireObjectCache;
 
+import javax.servlet.http.HttpServletResponse;
+import java.lang.reflect.Method;
+
 public class InvokeHandlerMethodInterceptor implements InstanceMethodsAroundInterceptor {
     @Override
     public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
@@ -42,6 +43,5 @@ public class InvokeHandlerMethodInterceptor implements InstanceMethodsAroundInte
 
     @Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
         Class<?>[] argumentsTypes, Throwable t) {
-
     }
 }

apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RequestMappingMethodInterceptor.java

@@ -19,16 +19,17 @@
 
 package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor;
 
-import java.lang.reflect.Method;
 import org.springframework.web.bind.annotation.RequestMapping;
 
+import java.lang.reflect.Method;
+
 /**
  * The <code>RequestMappingMethodInterceptor</code> only use the first mapping value.
  * it will inteceptor with <code>@RequestMapping</code>
  *
  * @author clevertension
  */
-public class RequestMappingMethodInterceptor extends AbstractMethodInteceptor {
+public class RequestMappingMethodInterceptor extends AbstractMethodInterceptor {
     @Override
     public String getRequestURL(Method method) {
         String requestURL = "";

apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RestMappingMethodInterceptor.java

@@ -19,12 +19,9 @@
 
 package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor;
 
+import org.springframework.web.bind.annotation.*;
+
 import java.lang.reflect.Method;
-import org.springframework.web.bind.annotation.DeleteMapping;
-import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.PatchMapping;
-import org.springframework.web.bind.annotation.PostMapping;
-import org.springframework.web.bind.annotation.PutMapping;
 
 /**
  * The <code>RestMappingMethodInterceptor</code> only use the first mapping value.
@@ -34,7 +31,7 @@ import org.springframework.web.bind.annotation.PutMapping;
  *
  * @author clevertension
  */
-public class RestMappingMethodInterceptor extends AbstractMethodInteceptor {
+public class RestMappingMethodInterceptor extends AbstractMethodInterceptor {
     @Override
     public String getRequestURL(Method method) {
         String requestURL = "";