You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@openwebbeans.apache.org by Vicente Rossello <co...@gmail.com> on 2020/07/28 09:19:58 UTC

Occasional NPE in buildRepeatableAnnotations

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?

Re: Occasional NPE in buildRepeatableAnnotations

Posted by Romain Manni-Bucau <rm...@gmail.com>.
done with https://issues.apache.org/jira/browse/OWB-1344

thanks again Vicente for the precise analysis.

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 à 12:48, vicente Rossello <ro...@gmail.com>
a écrit :

> Your change is good, I did the same thing without the computeIfAbsent, I
> prefer yours, simpler
>
> On Wed, Jul 29, 2020 at 12:29 PM Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
>> 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 <co...@gmail.com>
>> 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 <
>>> rmannibucau@gmail.com> 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 <
>>>> rmannibucau@gmail.com> 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 <
>>>>> cocorossello@gmail.com> 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?
>>>>>>
>>>>>>

Re: Occasional NPE in buildRepeatableAnnotations

Posted by vicente Rossello <ro...@gmail.com>.
Your change is good, I did the same thing without the computeIfAbsent, I
prefer yours, simpler

On Wed, Jul 29, 2020 at 12:29 PM Romain Manni-Bucau <rm...@gmail.com>
wrote:

> 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 <co...@gmail.com>
> 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 <rm...@gmail.com>
>> 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 <rm...@gmail.com>
>>> 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 <co...@gmail.com>
>>>> 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?
>>>>>
>>>>>

Re: Occasional NPE in buildRepeatableAnnotations

Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <co...@gmail.com> 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 <rm...@gmail.com>
> 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 <rm...@gmail.com>
>> 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 <co...@gmail.com>
>>> 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?
>>>>
>>>>

Re: Occasional NPE in buildRepeatableAnnotations

Posted by Vicente Rossello <co...@gmail.com>.
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 <rm...@gmail.com>
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 <rm...@gmail.com>
> 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 <co...@gmail.com>
>> 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?
>>>
>>>

Re: Occasional NPE in buildRepeatableAnnotations

Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <rm...@gmail.com> 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 <co...@gmail.com>
> 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?
>>
>>

Re: Occasional NPE in buildRepeatableAnnotations

Posted by Romain Manni-Bucau <rm...@gmail.com>.
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 <co...@gmail.com> 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?
>
>