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 2021/05/18 10:14:22 UTC
[GitHub] [skywalking] lujiajing1126 opened a new issue #6966: Discussion of ClassCast problem for RequestHolder
lujiajing1126 opened a new issue #6966:
URL: https://github.com/apache/skywalking/issues/6966
- Why do you submit this issue?
- [x] Question or discussion
- [x] Bug
- [ ] Requirement
- [ ] Feature or performance improvement
___
### Question
Discussion the `java.lang.ClassCastException` problem reported in issues #6554, #5817, #6648 and #6760.
And try to resolve this problem.
___
### Bug
- Which version of SkyWalking, OS, and JRE?
I suppose all the versions up to the latest Skywalking 8.5.0 are affected
- What happened?
In the `skywalking-api.log`, we may find,
```
ERROR 2021-05-18 17:40:02:384 http-nio-8080-exec-1 InstMethodsInter : class[class com.example.classloaderdemo.controller.HealthController] before method[health] intercept failure
java.lang.ClassCastException: org.apache.skywalking.apm.plugin.spring.mvc.commons.JavaxServletRequestHolder cannot be cast to org.apache.skywalking.apm.plugin.spring.mvc.commons.RequestHolder
at org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor.AbstractMethodInterceptor.beforeMethod(AbstractMethodInterceptor.java:100)
at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:76)
at com.example.classloaderdemo.controller.HealthController.health(HealthController.java)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:197)
at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:141)
at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:106)
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:894)
at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:808)
at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1060)
at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:962)
at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)
at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:626)
at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:733)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:227)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:202)
at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:97)
at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:542)
at org.apache.catalina.core.StandardHostValve.invoke$original$G32s1bTQ(StandardHostValve.java:143)
at org.apache.catalina.core.StandardHostValve.invoke$original$G32s1bTQ$accessor$ku9vVTW5(StandardHostValve.java)
at org.apache.catalina.core.StandardHostValve$auxiliary$7vA1kjbB.call(Unknown Source)
at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:86)
at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java)
at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)
at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:78)
at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:357)
at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:374)
at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:893)
at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1707)
at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
at java.lang.Thread.run(Thread.java:748)
```
As @zifeihan has concluded in https://github.com/apache/skywalking/issues/5817#issuecomment-751446913, the problem is easy to be understood. Just to take a glance of this story, the simplest way to reproduce the problem is to run a spring boot application with [`spring-boot-devtools`](https://docs.spring.io/spring-boot/docs/1.5.16.RELEASE/reference/html/using-boot-devtools.html) in IDEA.
```xml
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-devtools</artifactId>
<scope>runtime</scope>
<optional>true</optional>
</dependency>
```
with a controller,
```java
package com.example.classloaderdemo.controller;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;
@RestController
public class HealthController {
@GetMapping(path = "/healthz")
public String health() {
return "OK";
}
}
```
If we use [`Arthas`](https://arthas.aliyun.com/doc/index.html) to examine this particular class `org.apache.skywalking.apm.plugin.spring.mvc.commons.RequestHolder`, we can find the following results,
```
[arthas@22299]$ sc -d org.apache.skywalking.apm.plugin.spring.mvc.commons.RequestHolder
class-info org.apache.skywalking.apm.plugin.spring.mvc.commons.JavaxServletRequestHolder
code-source
name org.apache.skywalking.apm.plugin.spring.mvc.commons.JavaxServletRequestHolder
isInterface false
isAnnotation false
isEnum false
isAnonymousClass false
isArray false
isLocalClass false
isMemberClass false
isPrimitive false
isSynthetic false
simple-name JavaxServletRequestHolder
modifier public
annotation
interfaces org.apache.skywalking.apm.plugin.spring.mvc.commons.RequestHolder
super-class +-java.lang.Object
class-loader +-org.apache.skywalking.apm.agent.core.plugin.loader.AgentClassLoader@6ba76699
+-sun.misc.Launcher$AppClassLoader@18b4aac2
+-sun.misc.Launcher$ExtClassLoader@3feba861
classLoaderHash 6ba76699
class-info org.apache.skywalking.apm.plugin.spring.mvc.commons.RequestHolder
code-source
name org.apache.skywalking.apm.plugin.spring.mvc.commons.RequestHolder
isInterface true
isAnnotation false
isEnum false
isAnonymousClass false
isArray false
isLocalClass false
isMemberClass false
isPrimitive false
isSynthetic false
simple-name RequestHolder
modifier abstract,interface,public
annotation
interfaces
super-class
class-loader +-org.apache.skywalking.apm.agent.core.plugin.loader.AgentClassLoader@77f6031b
+-org.springframework.boot.devtools.restart.classloader.RestartClassLoader@6bce6c13
+-sun.misc.Launcher$AppClassLoader@18b4aac2
+-sun.misc.Launcher$ExtClassLoader@3feba861
classLoaderHash 77f6031b
class-info org.apache.skywalking.apm.plugin.spring.mvc.commons.RequestHolder
code-source
name org.apache.skywalking.apm.plugin.spring.mvc.commons.RequestHolder
isInterface true
isAnnotation false
isEnum false
isAnonymousClass false
isArray false
isLocalClass false
isMemberClass false
isPrimitive false
isSynthetic false
simple-name RequestHolder
modifier abstract,interface,public
annotation
interfaces
super-class
class-loader +-org.apache.skywalking.apm.agent.core.plugin.loader.AgentClassLoader@6ba76699
+-sun.misc.Launcher$AppClassLoader@18b4aac2
+-sun.misc.Launcher$ExtClassLoader@3feba861
classLoaderHash 6ba76699
Affect(row-cnt:3) cost in 30 ms.
```
1. `org.apache.skywalking.apm.plugin.spring.mvc.commons.JavaxServletRequestHolder` together with its parent `org.apache.skywalking.apm.plugin.spring.mvc.commons.RequestHolder` appear to be loaded by the `AgentClassLoader@6ba76699`. This happened when the spring internal class `org.springframework.web.method.HandlerMethod#getBean` was enhanced since the `HandlerMethod` class was loaded by `AppClassLoader`.
2. Afterwards, the `RestController` annotation is captured. However, the controller class `HealthController` is loaded by another classloader `org.springframework.boot.devtools.restart.classloader.RestartClassLoader@6bce6c13` which further causes the Skywalking agent to create a new instance of AgentClassLoader, i.e. `AgentClassLoader@77f6031b`.
Finally, since `RequestHolder` sits in both two classloaders, they are not the same one, thus not inter-changeable.
___
### Requirement or improvement
As a conclusion, the root-cause of the problem is the abstraction of `RequestHolder` which **exists in the plugin** and will be potentially loaded by different instances of `AgentClassLoader`.
But I suppose the abstraction can be fully removed since we only take advantage of it in `AbstractMethodInterceptor`.
If so, I believe this issue can be resolved, since either `javax.servlet.http.HttpServletRequest` or `org.springframework.http.server.reactive.ServerHttpResponse` can be only loaded by the same classloader due to the
parent delegation model, e.g. `AppClassLoader` or [`org.springframework.boot.loader.LaunchedURLClassLoader`](https://docs.spring.io/spring-boot/docs/current/api/org/springframework/boot/loader/LaunchedURLClassLoader.html) in spring boot execution environment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [skywalking] lujiajing1126 commented on issue #6966: Discussion of ClassCast problem for RequestHolder
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on issue #6966:
URL: https://github.com/apache/skywalking/issues/6966#issuecomment-843056889
> You have a very clear analysis of the issue. But I am not following your way of fixing it. Could you be more clear about this?
1. First, we remove the use of `RequestHolder` / `ResponseHolder`
2. We used to put the `RequestHolder` or `ResponseHolder` in the RuntimeContext. Now we directly put the native objects, i.e. `javax.servlet.http.HttpServletRequest` or `org.springframework.http.server.reactive.ServerHttpResponse` for the request.
3. Since the only place we use this holder object is (Pls correct me if I'm wrong)
https://github.com/apache/skywalking/blob/47b46f93caf20210995aba7fb6814ed82aeb8260/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java#L99-L100
We can check the type of the class on-site with `if`-condition, for example,
```java
Object request = ContextManager.getRuntimeContext().get(REQUEST_KEY_IN_RUNTIME_CONTEXT);
if (javax.servlet.http.HttpServletRequest.class.isAssignableFrom(request.getClass())) {
...
} else if (org.springframework.http.server.reactive.ServerHttpRequest.class.isAssignableFrom(request.getClass())) {
...
} else {
throw new UnsupportedOperationException("this line should not be reached");
}
```
Any better idea is welcome.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [skywalking] lujiajing1126 commented on issue #6966: Discussion of ClassCast problem for RequestHolder
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on issue #6966:
URL: https://github.com/apache/skywalking/issues/6966#issuecomment-843685571
> `isAssignableFrom` should be fine since JDK8. Performance is acceptable. You could run a benchmark to see which is better
>
> 1. This way, `isAssignableFrom`
> 2. Use 2 different keys in RuntimeContext and check NULL.
Intuitively, I may prefer the second one. I will run a benchmark to compare the ThreadLocal get and `isAssignableFrom` method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [skywalking] wu-sheng commented on issue #6966: Discussion of ClassCast problem for RequestHolder
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6966:
URL: https://github.com/apache/skywalking/issues/6966#issuecomment-843049571
You have a very clear analysis of the issue. But I am not following your way of fixing it. Could you be more clear about this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [skywalking] wu-sheng commented on issue #6966: Discussion of ClassCast problem for RequestHolder
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6966:
URL: https://github.com/apache/skywalking/issues/6966#issuecomment-843243334
`isAssignableFrom` should be fine since JDK8. Performance is acceptable. You could run a benchmark to see which is better
1. This way, `isAssignableFrom`
2. Use 2 different keys in RuntimeContext and check NULL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [skywalking] wu-sheng closed issue #6966: Discussion of ClassCast problem for RequestHolder
Posted by GitBox <gi...@apache.org>.
wu-sheng closed issue #6966:
URL: https://github.com/apache/skywalking/issues/6966
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [skywalking] lujiajing1126 edited a comment on issue #6966: Discussion of ClassCast problem for RequestHolder
Posted by GitBox <gi...@apache.org>.
lujiajing1126 edited a comment on issue #6966:
URL: https://github.com/apache/skywalking/issues/6966#issuecomment-843056889
> You have a very clear analysis of the issue. But I am not following your way of fixing it. Could you be more clear about this?
1. First, we remove the use of `RequestHolder` / `ResponseHolder`
2. We used to put the `RequestHolder` or `ResponseHolder` in the RuntimeContext. Now we directly put the native objects, i.e. `javax.servlet.http.HttpServletRequest` or `org.springframework.http.server.reactive.ServerHttpResponse` for the request.
3. Since the only place we use this holder object is in the `AbstractMethodInterceptor.java` (Pls correct me if I'm wrong)
https://github.com/apache/skywalking/blob/47b46f93caf20210995aba7fb6814ed82aeb8260/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java#L99-L100
We can check the type of the class on-site with `if`-condition, for example,
```java
Object request = ContextManager.getRuntimeContext().get(REQUEST_KEY_IN_RUNTIME_CONTEXT);
if (javax.servlet.http.HttpServletRequest.class.isAssignableFrom(request.getClass())) {
...
} else if (org.springframework.http.server.reactive.ServerHttpRequest.class.isAssignableFrom(request.getClass())) {
...
} else {
throw new UnsupportedOperationException("this line should not be reached");
}
```
Any better idea is welcome.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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
[GitHub] [skywalking] wu-sheng commented on issue #6966: Discussion of ClassCast problem for RequestHolder
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6966:
URL: https://github.com/apache/skywalking/issues/6966#issuecomment-843686117
> > `isAssignableFrom` should be fine since JDK8. Performance is acceptable. You could run a benchmark to see which is better
> >
> > 1. This way, `isAssignableFrom`
> > 2. Use 2 different keys in RuntimeContext and check NULL.
>
> Intuitively, I may prefer the second one. I will run a benchmark to compare the ThreadLocal get and `isAssignableFrom` method.
Agree, let's run it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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