Oki, it is https://github.com/apache/tomee/blob/8f6bd834a37a8745331c6580c458813e713db92c/container/openejb-core/src/main/java/org/apache/openejb/cdi/transactional/InterceptorBase.java#L92 which is really a runtime call and I think the original author wanted to keep it lazy since it is only about the error case (;)). Do you have another fix than the concurrent map one or should I push it as this - also happy to go through a PR if you have something handy?
Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <https://rmannibucau.metawerx.net/> | Old Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book <https://www.packtpub.com/application-development/java-ee-8-high-performance> Le mer. 29 juil. 2020 à 11:46, Vicente Rossello <[email protected]> a écrit : > Hi, I did this change (similar) in our build yesterday and everything > works fine. > > The whole stacktrace is: > > java.util.concurrent.ExecutionException: java.lang.RuntimeException: > java.lang.NullPointerException: Cannot invoke > "java.util.Optional.isPresent()" because "repeatableMethod" is null > at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122) > at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:205) > at org.apache.openejb.threads.future.CUFuture.get(CUFuture.java:60) > at > com.infratr2.interceptor.async.FutureDelegator.get(FutureDelegator.java:23) > at > com.tr2.test.TestDataRepository.saveConcurrentDatasheets(TestDataRepository.java:232) > at > com.tr2.test.TestDataRepository$$OwbInterceptProxy0.saveConcurrentDatasheets(com/tr2/test/TestDataRepository.java) > at > com.tr2.test.TestDataRepository$$OwbNormalScopeProxy0.saveConcurrentDatasheets(com/tr2/test/TestDataRepository.java) > at > com.tr2.web.business.HotelDataSheetServiceIT.save_concurrent_hoteldatasheets(HotelDataSheetServiceIT.java:40) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:564) > at > org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:686) > at > org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60) > at > org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131) > at > org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149) > at > org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140) > at > org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84) > at > org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115) > at > org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105) > at > org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106) > at > org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64) > at > org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45) > at > org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37) > at > org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104) > at > org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98) > at > org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:212) > at > org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) > at > org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:208) > at > org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:137) > at > org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:71) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135) > at > org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125) > at > org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123) > at > org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80) > at > org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171) > at > org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:115) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139) > at > org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125) > at > org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123) > at > org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80) > at > org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171) > at > org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.executeNonConcurrentTasks(ForkJoinPoolHierarchicalTestExecutorService.java:141) > at > org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:121) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139) > at > org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125) > at > org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123) > at > org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122) > at > org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80) > at > org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171) > at > java.base/java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:189) > at > java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290) > at > java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1016) > at > java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1665) > at > java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1598) > at > java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183) > Caused by: java.lang.RuntimeException: java.lang.NullPointerException: Cannot > invoke "java.util.Optional.isPresent()" because "repeatableMethod" is null > at > org.apache.webbeans.portable.AnnotatedElementFactory.newAnnotatedType(AnnotatedElementFactory.java:204) > at > org.apache.webbeans.container.BeanManagerImpl.createAnnotatedType(BeanManagerImpl.java:585) > at > org.apache.webbeans.container.InjectableBeanManager.createAnnotatedType(InjectableBeanManager.java:91) > at > org.apache.openejb.cdi.transactional.InterceptorBase.intercept(InterceptorBase.java:92) > at > org.apache.openejb.cdi.transactional.RequiredInterceptor.intercept(RequiredInterceptor.java:35) > at jdk.internal.reflect.GeneratedMethodAccessor125.invoke(Unknown > Source) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:564) > at > org.apache.webbeans.component.InterceptorBean.intercept(InterceptorBean.java:136) > at > org.apache.webbeans.intercept.InterceptorInvocationContext.proceed(InterceptorInvocationContext.java:65) > at > org.apache.webbeans.intercept.DefaultInterceptorHandler.invoke(DefaultInterceptorHandler.java:139) > at > com.tr2.engine.business.HotelDataSheetRepository$$OwbInterceptProxy0.save(com/tr2/engine/business/HotelDataSheetRepository.java) > at > com.tr2.engine.business.HotelDataSheetRepository$$OwbNormalScopeProxy0.save(com/tr2/engine/business/HotelDataSheetRepository.java) > at > com.tr2.engine.HotelDataSheetService.saveDataSheet(HotelDataSheetService.java:44) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:564) > at > org.apache.webbeans.intercept.AbstractInvocationContext.directProceed(AbstractInvocationContext.java:113) > at > org.apache.webbeans.intercept.AbstractInvocationContext.proceed(AbstractInvocationContext.java:106) > at > org.apache.webbeans.intercept.InterceptorInvocationContext.proceed(InterceptorInvocationContext.java:78) > at > org.apache.openejb.cdi.transactional.InterceptorBase.intercept(InterceptorBase.java:67) > at > org.apache.openejb.cdi.transactional.RequiredInterceptor.intercept(RequiredInterceptor.java:35) > at jdk.internal.reflect.GeneratedMethodAccessor125.invoke(Unknown > Source) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:564) > at > org.apache.webbeans.component.InterceptorBean.intercept(InterceptorBean.java:136) > at > org.apache.webbeans.intercept.InterceptorInvocationContext.proceed(InterceptorInvocationContext.java:65) > at > org.apache.webbeans.intercept.DefaultInterceptorHandler.invoke(DefaultInterceptorHandler.java:139) > at > com.tr2.engine.HotelDataSheetService$$OwbInterceptProxy0.saveDataSheet(com/tr2/engine/HotelDataSheetService.java) > at > com.tr2.engine.HotelDataSheetService$$OwbNormalScopeProxy0.saveDataSheet(com/tr2/engine/HotelDataSheetService.java) > at > com.tr2.test.TestDataRepository.saveDatasheet(TestDataRepository.java:245) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:564) > at > org.apache.webbeans.intercept.AbstractInvocationContext.directProceed(AbstractInvocationContext.java:113) > at > org.apache.webbeans.intercept.AbstractInvocationContext.proceed(AbstractInvocationContext.java:106) > at > org.apache.webbeans.intercept.InterceptorInvocationContext.proceed(InterceptorInvocationContext.java:78) > at > com.tr2.engine.base.monitoring.JavaMelodyUtil.lambda$intercept$1(JavaMelodyUtil.java:91) > at > com.tr2.engine.base.monitoring.JavaMelodyUtil.intercept(JavaMelodyUtil.java:54) > at > com.tr2.engine.base.monitoring.JavaMelodyUtil.intercept(JavaMelodyUtil.java:89) > at > com.tr2.engine.base.monitoring.JavaMelodyUtil.interceptAsync(JavaMelodyUtil.java:103) > at > com.infratr2.interceptor.async.AsyncInterceptor.asyncExecution(AsyncInterceptor.java:67) > at > com.infratr2.interceptor.async.AsyncInterceptor.lambda$aroundInvoke$0(AsyncInterceptor.java:47) > at org.apache.openejb.threads.task.CUTask.invoke(CUTask.java:100) > at org.apache.openejb.threads.task.CUCallable.call(CUCallable.java:31) > at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) > at > java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1130) > at > java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:630) > at java.base/java.lang.Thread.run(Thread.java:832) > Caused by: java.lang.NullPointerException: Cannot invoke > "java.util.Optional.isPresent()" because "repeatableMethod" is null > at > org.apache.webbeans.portable.AbstractAnnotated.buildRepeatableAnnotations(AbstractAnnotated.java:107) > at > org.apache.webbeans.portable.AbstractAnnotated.setAnnotations(AbstractAnnotated.java:166) > at > org.apache.webbeans.portable.AnnotatedTypeImpl.<init>(AnnotatedTypeImpl.java:76) > at > org.apache.webbeans.portable.AnnotatedElementFactory.newAnnotatedType(AnnotatedElementFactory.java:183) > ... 50 more > > > The test is an application composer with CDI and jpa enabled. > > > > On Wed, Jul 29, 2020 at 8:54 AM Romain Manni-Bucau <[email protected]> > wrote: > >> Reviewed a bit more this part and think we can just drop the 2 structure >> state to only use the concurrent map we already have, the computeIfAbsent >> overhead will be negligible compared to the simple contains and it will >> solve by construction the locking issue: >> >> diff --git >> a/webbeans-impl/src/main/java/org/apache/webbeans/annotation/AnnotationManager.java >> b/webbeans-impl/src/main/java/org/apache/webbeans/annotation/AnnotationManager.java >> index 24033f4ec..b5442c446 100644 >> --- >> a/webbeans-impl/src/main/java/org/apache/webbeans/annotation/AnnotationManager.java >> +++ >> b/webbeans-impl/src/main/java/org/apache/webbeans/annotation/AnnotationManager.java >> @@ -62,7 +62,7 @@ import java.util.Map; >> import java.util.Optional; >> import java.util.Set; >> import java.util.concurrent.ConcurrentHashMap; >> -import java.util.concurrent.CopyOnWriteArraySet; >> +import java.util.concurrent.ConcurrentMap; >> >> /** >> * Manages annotation usage by classes in this application. >> @@ -74,8 +74,7 @@ public final class AnnotationManager >> private Map<Class<? extends Annotation>, Boolean> >> checkedStereotypeAnnotations = >> new ConcurrentHashMap<>(); >> >> - private CopyOnWriteArraySet<Class<?>> repeatableMethodCheckedTypes = >> new CopyOnWriteArraySet<>(); >> - private Map<Class<?>, Optional<Method>> repeatableMethodCache = new >> ConcurrentHashMap<>(); >> + private ConcurrentMap<Class<?>, Optional<Method>> >> repeatableMethodCache = new ConcurrentHashMap<>(); >> >> private final BeanManagerImpl beanManagerImpl; >> private final WebBeansContext webBeansContext; >> @@ -938,23 +937,12 @@ public final class AnnotationManager >> >> public void clearCaches() >> { >> - repeatableMethodCheckedTypes.clear(); >> repeatableMethodCache.clear(); >> } >> >> public Optional<Method> getRepeatableMethod(Class<?> type) >> { >> - if (repeatableMethodCheckedTypes.contains(type)) >> - { >> - return repeatableMethodCache.get(type); >> - } >> - >> - Optional<Method> method = >> Optional.ofNullable(resolveRepeatableMethod(type)); >> - >> - repeatableMethodCheckedTypes.add(type); >> - repeatableMethodCache.put(type, method); // don't put null here! >> - >> - return method; >> + return repeatableMethodCache.computeIfAbsent(type, it -> >> Optional.ofNullable(resolveRepeatableMethod(it))); >> } >> >> protected Method resolveRepeatableMethod(Class<?> type) >> >> >> still needs the stack to validate the hypothesis but even if it does not >> fix the original issue it looks better in terms of impl, wdyt? >> Romain Manni-Bucau >> @rmannibucau <https://twitter.com/rmannibucau> | Blog >> <https://rmannibucau.metawerx.net/> | Old Blog >> <http://rmannibucau.wordpress.com> | Github >> <https://github.com/rmannibucau> | LinkedIn >> <https://www.linkedin.com/in/rmannibucau> | Book >> <https://www.packtpub.com/application-development/java-ee-8-high-performance> >> >> >> Le mar. 28 juil. 2020 à 21:39, Romain Manni-Bucau <[email protected]> >> a écrit : >> >>> Hi >>> >>> It is a startup method normally so synchro should be useless until you >>> abuse of createAnnotatedType at runtime - which can be another library bug >>> ;). >>> >>> Would be great to have more of the stacktrace to check it - which means >>> we would lock only this specific runtime api and not startup ones (we have >>> that state and there are different methods calling it). >>> >>> Hope it makes sense >>> >>> Le mar. 28 juil. 2020 à 11:20, Vicente Rossello <[email protected]> >>> a écrit : >>> >>>> Hi, >>>> >>>> In our CI server I can see an occasional NPE: >>>> >>>> Caused by: java.lang.NullPointerException: Cannot invoke >>>> "java.util.Optional.isPresent()" because "repeatableMethod" is null >>>> at >>>> org.apache.webbeans.portable.AbstractAnnotated.buildRepeatableAnnotations(AbstractAnnotated.java:107) >>>> at >>>> org.apache.webbeans.portable.AbstractAnnotated.setAnnotations(AbstractAnnotated.java:166) >>>> at >>>> org.apache.webbeans.portable.AnnotatedTypeImpl.<init>(AnnotatedTypeImpl.java:76) >>>> at >>>> org.apache.webbeans.portable.AnnotatedElementFactory.newAnnotatedType(AnnotatedElementFactory.java:183) >>>> >>>> >>>> (It's a test that spans some threads and persists some concurrent data). >>>> >>>> >>>> It happens once a day at most, and I think it happens lately since I >>>> upgraded to 2.0.16 or 17 (I have no idea why because this hasn't changed >>>> in a while, I can't confirm this) >>>> >>>> >>>> I've been unable to reproduce it locally neither. >>>> >>>> >>>> The thing is that looking at AnnotationManager.java: >>>> >>>> >>>> public Optional<Method> getRepeatableMethod(Class<?> type) >>>> { >>>> if (repeatableMethodCheckedTypes.contains(type)) >>>> { >>>> return repeatableMethodCache.get(type); >>>> } >>>> >>>> Optional<Method> method = >>>> Optional.ofNullable(resolveRepeatableMethod(type)); >>>> >>>> repeatableMethodCheckedTypes.add(type); >>>> repeatableMethodCache.put(type, method); // don't put null here! >>>> >>>> return method; >>>> } >>>> >>>> >>>> The repeatableMethodCheckedTypes can have the value and the >>>> repeatableMethodCache empty at the same time, which I guess is what's >>>> happening here. >>>> >>>> >>>> A solution can be to synchronize the add and put, or the >>>> repeatableMethodCheckedTypes can be removed (it's not used anywhere else, >>>> I guess it's there for performance reasons?) >>>> >>>> WDYT? >>>> >>>>
