NPE in serialize EnhanceRequireObjectCache

[candyleer]

Please answer these questions before submitting your issue.

• Why do you submit this issue?
• Question or discussion
• Bug
• Requirement
• Feature or performance improvement

---

Question

• What do you want to know?

---

Bug

• Which version of SkyWalking, OS and JRE?
  latest master,mac os ,jdk8

• Which company or project?

• What happen?
  in some cases,end user may want to log the result of web api,so they may use the aop in spring to completed this feature,they may don't know the enhanced method,so they add this aop in all method.eg.

@Around("execution(* com.example.test.TestApplication.*(..))")
    public Object aspectAroundHttpInvoke(ProceedingJoinPoint proceedingJoinPoint) throws Throwable {
        Object proceed = proceedingJoinPoint.proceed();
        String resultString = JSONObject.toJSONString(proceed);
        System.out.println(resultString);
        System.out.println(objectMapper.writeValueAsString(proceed));
        System.out.println(new Gson().toJson(proceed));
        return proceed;
    }

maybe this work well before import skywalking,but if some users have this code, the trace may be interrupted by an NPE exception like this:

ERROR 2018-04-24 22:34:21 InstMethodsInter :  class[class com.example.test.TestApplication$$EnhancerBySpringCGLIB$$f89779b2] after method[hello] intercept failure
java.lang.NullPointerException
	at org.apache.skywalking.apm.plugin.spring.mvc.commons.EnhanceRequireObjectCache.getHttpServletResponse(EnhanceRequireObjectCache.java:36)
	at org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor.AbstractMethodInterceptor.afterMethod(AbstractMethodInterceptor.java:77)
	at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:105)
	at com.example.test.TestApplication.hello(TestApplication.java)
	at com.example.test.TestApplication$$FastClassBySpringCGLIB$$a7df4b33.invoke(<generated>)
	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:204)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:738)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:157)
	at org.springframework.aop.aspectj.MethodInvocationProceedingJoinPoint.proceed(MethodInvocationProceedingJoinPoint.java:85)
	at com.example.test.LogAspect.aspectAroundHttpInvoke(LogAspect.java:36)
	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.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:629)
	at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:618)
	at org.springframework.aop.aspectj.AspectJAroundAdvice.invoke(AspectJAroundAdvice.java:70)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:673)
	at com.example.test.TestApplication$$EnhancerBySpringCGLIB$$9809f3e0.hello(<generated>)
	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:205)
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest$original$NRyKSdT0(InvocableHandlerMethod.java:133)
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest$original$NRyKSdT0$accessor$7MICXcvU(InvocableHandlerMethod.java)
	at org.springframework.web.method.support.InvocableHandlerMethod$auxiliary$9AXEpcNm.call(Unknown Source)
	at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:93)
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java)
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:97)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:827)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:738)
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:85)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:967)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:901)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:970)
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:861)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:635)
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:846)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:742)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:99)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.springframework.web.filter.HttpPutFormContentFilter.doFilterInternal(HttpPutFormContentFilter.java:108)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.springframework.web.filter.HiddenHttpMethodFilter.doFilterInternal(HiddenHttpMethodFilter.java:81)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:197)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.apache.catalina.core.StandardWrapperValve.invoke$original$8QnrZ6bk(StandardWrapperValve.java:199)
	at org.apache.catalina.core.StandardWrapperValve.invoke$original$8QnrZ6bk$accessor$Dc1SU7Jj(StandardWrapperValve.java)
	at org.apache.catalina.core.StandardWrapperValve$auxiliary$YaGN5PL0.call(Unknown Source)
	at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:93)
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java)
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:96)
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:478)
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:140)
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:81)
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:342)
	at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:803)
	at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:66)
	at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:868)
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1459)
	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)

the cause of this issuce is:

• end user want to serialize and print( or store) and result of each method in controller
• we have add an enhanced method named setSkyWalkingDynamicField and getSkyWalkingDynamicField
• in agent interceptor org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor.GetBeanInterceptor#afterMethod ,getSkyWalkingDynamicField method is called by controller instance, so the custom aop will intercept it
• the result type of getSkyWalkingDynamicField() is EnhanceRequireObjectCache
• serialize framework like fastjson,gson,jackson will serialize the method which name's prefix is get,like getHttpServletResponse()
• when serialize, the method will throw an NPE (nativeWebRequest has not been set value), so trace interrupted and throw the exception above.

Fortunately, this issue will not blocking user's business logic code,only affect the trace data.
So, to be compatible with this situation, we can

• rename the method name in EnhanceRequireObjectCache,replace get* to other
• complete the logic judgment of method getHttpServletResponse,make sure the method will not throw NPE in any situation.

---

Requirement or improvement

• Please describe about your requirements or improvement suggestions.

[wu-sheng]

This will be a long time discussion. :)

First of all, these enhanced methods, #get/#set of dynamicField, are for performance. It is complex to end user, but really necessary. Also I agree with you, if some one uses AOP for their bean classes, and try to serialize them, which may be highly possible to trigger this issue.

So I think the solution for this, should not just fix bugs. Instaed I suggest we provide a new patch for Spring AOP.

@candyleer Are you aware of that, we are providing a Spring core patch, to make sure a normal class would be transferred to a dynamic interface instance, because SkyWalking add an interface for each enhanced class(That interface includes #get/#set methods of dynamicField). SkyWalking spring patch forces the Spring core to use CglibProxyFactory#createCglibProxy instead of JDKProxy, even the class has the EnhancedInstance interface.

As same as Spring core patch, we should provide a Spring aop patch. This patch will filter all getSkyWalkingDynamicField and setSkyWalkingDynamicField from the EnhancedInstance's implementor classes, when they are matched by some AOP mechanism.

Could you start to find the Spring AOP core codes, to find out how Spring filters classes and methods?

[candyleer]

@wu-sheng good idea, i'll prepare to find the AOP code to filter enhanced method

[wu-sheng]

fixed.