Fix a tracing context leak in SpringMVC plugin (#34)
* Once the springMVC span is created in beforeMethod, it must be stopped in afterMethod. Or it may cause memory leak.
Authored-by: furao <furao@didiglobal.com>
diff --git a/CHANGES.md b/CHANGES.md
index f39b0b9..be31fc2 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -21,6 +21,8 @@
* Add benchmark result for `exception-ignore` plugin and polish plugin guide.
* Provide Alibaba Druid database connection pool plugin.
* Provide HikariCP database connection pool plugin.
+* fix the bug that springMVC plugin will not stop span when exception throws.
+
#### Documentation
diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java
index 9e8493a..46a1742 100644
--- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java
+++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java
@@ -198,56 +198,58 @@
Object request = runtimeContext.get(REQUEST_KEY_IN_RUNTIME_CONTEXT);
if (request != null) {
- StackDepth stackDepth = (StackDepth) runtimeContext.get(CONTROLLER_METHOD_STACK_DEPTH);
- if (stackDepth == null) {
- throw new IllegalMethodStackDepthException();
- } else {
- stackDepth.decrement();
- }
-
- AbstractSpan span = ContextManager.activeSpan();
-
- if (stackDepth.depth() == 0) {
- Object response = runtimeContext.get(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
- if (response == null) {
- throw new ServletResponseNotFoundException();
+ try {
+ StackDepth stackDepth = (StackDepth) runtimeContext.get(CONTROLLER_METHOD_STACK_DEPTH);
+ if (stackDepth == null) {
+ throw new IllegalMethodStackDepthException();
+ } else {
+ stackDepth.decrement();
}
- Integer statusCode = null;
+ AbstractSpan span = ContextManager.activeSpan();
- if (IS_SERVLET_GET_STATUS_METHOD_EXIST && HttpServletResponse.class.isAssignableFrom(response.getClass())) {
- statusCode = ((HttpServletResponse) response).getStatus();
- } else if (ServerHttpResponse.class.isAssignableFrom(response.getClass())) {
- if (IS_SERVLET_GET_STATUS_METHOD_EXIST) {
- statusCode = ((ServerHttpResponse) response).getRawStatusCode();
+ if (stackDepth.depth() == 0) {
+ Object response = runtimeContext.get(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
+ if (response == null) {
+ throw new ServletResponseNotFoundException();
}
- Object context = runtimeContext.get(REACTIVE_ASYNC_SPAN_IN_RUNTIME_CONTEXT);
- if (context != null) {
- ((AbstractSpan[]) context)[0] = span.prepareForAsync();
+
+ Integer statusCode = null;
+
+ if (IS_SERVLET_GET_STATUS_METHOD_EXIST && HttpServletResponse.class.isAssignableFrom(response.getClass())) {
+ statusCode = ((HttpServletResponse) response).getStatus();
+ } else if (ServerHttpResponse.class.isAssignableFrom(response.getClass())) {
+ if (IS_SERVLET_GET_STATUS_METHOD_EXIST) {
+ statusCode = ((ServerHttpResponse) response).getRawStatusCode();
+ }
+ Object context = runtimeContext.get(REACTIVE_ASYNC_SPAN_IN_RUNTIME_CONTEXT);
+ if (context != null) {
+ ((AbstractSpan[]) context)[0] = span.prepareForAsync();
+ }
+ }
+
+ if (statusCode != null && statusCode >= 400) {
+ span.errorOccurred();
+ Tags.HTTP_RESPONSE_STATUS_CODE.set(span, statusCode);
+ }
+
+ runtimeContext.remove(REACTIVE_ASYNC_SPAN_IN_RUNTIME_CONTEXT);
+ runtimeContext.remove(REQUEST_KEY_IN_RUNTIME_CONTEXT);
+ runtimeContext.remove(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
+ runtimeContext.remove(CONTROLLER_METHOD_STACK_DEPTH);
+ }
+
+ // Active HTTP parameter collection automatically in the profiling context.
+ if (!SpringMVCPluginConfig.Plugin.SpringMVC.COLLECT_HTTP_PARAMS && span.isProfiling()) {
+ if (HttpServletRequest.class.isAssignableFrom(request.getClass())) {
+ RequestUtil.collectHttpParam((HttpServletRequest) request, span);
+ } else if (ServerHttpRequest.class.isAssignableFrom(request.getClass())) {
+ RequestUtil.collectHttpParam((ServerHttpRequest) request, span);
}
}
-
- if (statusCode != null && statusCode >= 400) {
- span.errorOccurred();
- Tags.HTTP_RESPONSE_STATUS_CODE.set(span, statusCode);
- }
-
- runtimeContext.remove(REACTIVE_ASYNC_SPAN_IN_RUNTIME_CONTEXT);
- runtimeContext.remove(REQUEST_KEY_IN_RUNTIME_CONTEXT);
- runtimeContext.remove(RESPONSE_KEY_IN_RUNTIME_CONTEXT);
- runtimeContext.remove(CONTROLLER_METHOD_STACK_DEPTH);
+ } finally {
+ ContextManager.stopSpan();
}
-
- // Active HTTP parameter collection automatically in the profiling context.
- if (!SpringMVCPluginConfig.Plugin.SpringMVC.COLLECT_HTTP_PARAMS && span.isProfiling()) {
- if (HttpServletRequest.class.isAssignableFrom(request.getClass())) {
- RequestUtil.collectHttpParam((HttpServletRequest) request, span);
- } else if (ServerHttpRequest.class.isAssignableFrom(request.getClass())) {
- RequestUtil.collectHttpParam((ServerHttpRequest) request, span);
- }
- }
-
- ContextManager.stopSpan();
}
return ret;
