Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NPE in AnnotationBasedPersistentProperty.isAnnotationPresent() because the annotation cache is not thread-safe [DATACMNS-1271] #1709

Closed
spring-projects-issues opened this issue Mar 1, 2018 · 3 comments

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Mar 1, 2018

Vojtech Toman opened DATACMNS-1271 and commented

In our application, during a highly concurrent ingestion, we encountered the NPE below. It seems that AnnotationBasedPersistentProperty.annotationCache (which is an ordinary HashMap) may misbehave when accessed concurrently and return null which then causes an NPE in org.springframework.data.mapping.model.AnnotationBasedPersistentProperty.isAnnotationPresent(). The relevant code in AnnotationBasedPersistentProperty.isAnnotationPresent() looks as follows:

if (annotationCache != null && annotationCache.containsKey(annotationType)) {
  return (Optional<A>) annotationCache.get(annotationType);
}

Based on, for example, this StackOverflow answer, a rehash of the hash map as a result of a concurrent insertion may lead to the situation where containsKey(key) returns true but the subsequent get(key) returns null. I believe this is what is going on here. The issue was observed with spring-data 2.0.3 but expect that it is present also in 2.0.5.

java.lang.NullPointerException: null
	at org.springframework.data.mapping.model.AnnotationBasedPersistentProperty.isAnnotationPresent(AnnotationBasedPersistentProperty.java:282)
	at org.springframework.data.mapping.model.BasicPersistentEntity.lambda$doFindPersistentProperty$1(BasicPersistentEntity.java:283)
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:174)
	at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1374)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
	at org.springframework.data.mapping.model.BasicPersistentEntity.doFindPersistentProperty(BasicPersistentEntity.java:284)
	at java.util.Map.computeIfAbsent(Map.java:957)
	at org.springframework.data.mapping.model.BasicPersistentEntity.getPersistentProperties(BasicPersistentEntity.java:277)
	at org.springframework.data.mapping.PersistentEntity.getPersistentProperty(PersistentEntity.java:172)
	at org.springframework.data.auditing.MappingAuditableBeanWrapperFactory$MappingAuditingMetadata.<init>(MappingAuditableBeanWrapperFactory.java:113)
	at org.springframework.data.auditing.MappingAuditableBeanWrapperFactory.lambda$null$0(MappingAuditableBeanWrapperFactory.java:81)
	at java.util.HashMap.computeIfAbsent(HashMap.java:1126)
	at org.springframework.data.auditing.MappingAuditableBeanWrapperFactory.lambda$null$1(MappingAuditableBeanWrapperFactory.java:80)
	at java.util.Optional.map(Optional.java:215)
	at org.springframework.data.auditing.MappingAuditableBeanWrapperFactory.lambda$getBeanWrapperFor$3(MappingAuditableBeanWrapperFactory.java:78)
	at java.util.Optional.flatMap(Optional.java:241)
	at org.springframework.data.auditing.MappingAuditableBeanWrapperFactory.getBeanWrapperFor(MappingAuditableBeanWrapperFactory.java:70)
	at org.springframework.data.auditing.AuditingHandler.isAuditable(AuditingHandler.java:157)
	at org.springframework.data.auditing.IsNewAwareAuditingHandler.markAudited(IsNewAwareAuditingHandler.java:80)
	at internal.org.springframework.data.xdb.mapping.event.AuditingEventListener.onApplicationEvent(AuditingEventListener.java:32)
	at internal.org.springframework.data.xdb.mapping.event.AuditingEventListener.onApplicationEvent(AuditingEventListener.java:14)
	at org.springframework.context.event.SimpleApplicationEventMulticaster.doInvokeListener(SimpleApplicationEventMulticaster.java:172)
	at org.springframework.context.event.SimpleApplicationEventMulticaster.invokeListener(SimpleApplicationEventMulticaster.java:165)
	at org.springframework.context.event.SimpleApplicationEventMulticaster.multicastEvent(SimpleApplicationEventMulticaster.java:139)
	at org.springframework.context.support.AbstractApplicationContext.publishEvent(AbstractApplicationContext.java:399)
	at org.springframework.context.support.AbstractApplicationContext.publishEvent(AbstractApplicationContext.java:353)
	at internal.org.springframework.data.xdb.core.XdbTemplate.maybeEmitEvent(XdbTemplate.java:294)
	...

Affects: 1.13.10 (Ingalls SR10), 2.0.5 (Kay SR5)

Backported to: 2.0.6 (Kay SR6), 1.13.11 (Ingalls SR11)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 1, 2018

Mark Paluch commented

Good catch, that should be rather ConcurrentHashMap with a slightly different fetching

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 1, 2018

Oliver Drotbohm commented

That's fixed. Feel free to give the snapshots a try

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 2, 2018

Oliver Drotbohm commented

I've decided to back-port this to 1.x as well as we basically had the same problem there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants