Thread-safetey issue in annotation detection in BasicPersistentEntity [DATACMNS-1210]

[spring-projects-issues]

Philipp Röben opened DATACMNS-1210 and commented

I found the problem using spring-data-redis but it I think it could apply to all spring-data projects
I was not able to file a bug in spring-data-mapping because there was no assignee.

You can reproduce the issue easily:

@Test
    void testThreadsafe(@Autowired MyPOJORepository repository){
        List<MyPOJO> all = new ArrayList<>();
        for(int i=0; i<10; i++){
            MyPOJO pojo = new MyPOJO();
            pojo.setMyProp("test"+i);
            all.add(pojo);
        }
        all.parallelStream().forEach( p -> repository.save(p));
    }

MyPOJO is a normal POJO annotated with Redis/Spring Data annotations:

@RedisHash("myPojo")
public class MyPOJO {
    @Id
    private String myProp;
... rest omitted

the Repository is quite simple:

public interface MyPOJORepository extends CrudRepository<MyPOJO, String> {}

When firing up the test, I get:

Caused by: java.util.ConcurrentModificationException
	at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1139)
	at org.springframework.data.mapping.model.BasicPersistentEntity.doFindAnnotation(BasicPersistentEntity.java:394)
	at org.springframework.data.mapping.model.BasicPersistentEntity.findAnnotation(BasicPersistentEntity.java:379)
	at org.springframework.data.redis.core.mapping.RedisMappingContext$ConfigAwareTimeToLiveAccessor.resolveDefaultTimeOut(RedisMappingContext.java:293)
	at org.springframework.data.redis.core.mapping.RedisMappingContext$ConfigAwareTimeToLiveAccessor.getTimeToLive(RedisMappingContext.java:220)
	at org.springframework.data.redis.core.convert.MappingRedisConverter.write(MappingRedisConverter.java:392)
	at org.springframework.data.redis.core.convert.MappingRedisConverter.write(MappingRedisConverter.java:118)
	at org.springframework.data.redis.core.RedisKeyValueAdapter.put(RedisKeyValueAdapter.java:206)
	at org.springframework.data.keyvalue.core.KeyValueTemplate.lambda$update$1(KeyValueTemplate.java:204)
	at org.springframework.data.keyvalue.core.KeyValueTemplate.execute(KeyValueTemplate.java:343)
	... 30 more

In Sourcecode you can see that in BasicPersistenceEntity:

this.annotationCache = new HashMap<>();

... is just a simple map, no ConcurrentHashmap and no other means of making this part threadsafe.

When I do the Test in single Thread mode there is no problem.

Basically this means all the Spring Data Repositories are not threadsafe!

---

Affects: 2.0.1 (Kay SR1)

Issue Links:

• DATACMNS-1364 BasicPersistentEntity.getPersistentProperty(…) returns the unchecked value in ConcurrentReferenceHashMap

Referenced from: pull request #259, and commits 309cdc5, 24c1b82, 8026f8a, 8185885

Backported to: 2.0.2 (Kay SR2)

[spring-projects-issues]

Philipp Röben commented

Thanks guys for fixong this so quickly!
(y)