Possible to Encounter ConcurrentModificationException in SessionFactoryImpl

[seabamirum]

When running load tests with 40 threads using the Neo4j driver version 5.9.0, I encountered a ConcurrentModificationException in the toDistinctSet() method of SessionFactoryImpl.

Proposed Fix: on line 74 in the toDistinctSet method in SessionFactoryImpl, replace HashSet<>() with Collections.synchronizedSet(new HashSet<>())

Original Stack Trace:
		at java.base/java.util.HashMap$HashIterator.nextNode(HashMap.java:1605) ~[na:na]
		at java.base/java.util.HashMap$KeyIterator.next(HashMap.java:1628) ~[na:na]
		at java.base/java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1054) ~[na:na]
		at org.neo4j.driver.internal.SessionFactoryImpl.toDistinctSet(SessionFactoryImpl.java:76) ~[neo4j-java-driver-5.9.0.jar:5.9.0-1250b2d94997ae49e5a623c52f61636505185057]
		at org.neo4j.driver.internal.SessionFactoryImpl.newInstance(SessionFactoryImpl.java:64) ~[neo4j-java-driver-5.9.0.jar:5.9.0-1250b2d94997ae49e5a623c52f61636505185057]
		at org.neo4j.driver.internal.InternalDriver.newSession(InternalDriver.java:218) ~[neo4j-java-driver-5.9.0.jar:5.9.0-1250b2d94997ae49e5a623c52f61636505185057]
		at org.neo4j.driver.internal.InternalDriver.session(InternalDriver.java:108) ~[neo4j-java-driver-5.9.0.jar:5.9.0-1250b2d94997ae49e5a623c52f61636505185057]
		at org.neo4j.driver.Driver.session(Driver.java:212) ~[neo4j-java-driver-5.9.0.jar:5.9.0-1250b2d94997ae49e5a623c52f61636505185057]
		at org.springframework.data.neo4j.core.DefaultReactiveNeo4jClient.lambda$getQueryRunner$0(DefaultReactiveNeo4jClient.java:94) ~[spring-data-neo4j-7.1.2.jar:7.1.2]
		at reactor.core.publisher.MonoSupplier$MonoSupplierSubscription.request(MonoSupplier.java:126) ~[reactor-core-3.5.8.jar:3.5.8]

[injectives]

Hello.

It actually looks like something is modifying the bookmarks given to the SessionConfig, which causes the following to fail:

for (var bookmark : bookmarks) {

[seabamirum]

Yes, you're right! I'm not sure why I thought it was the HashSet since the exception is clear that it's a HashMap.

java.util.ConcurrentModificationException: null
	at java.base/java.util.HashMap$HashIterator.nextNode(HashMap.java:1605) ~[na:na]

[injectives]

Well, the main issue seems to be that the bookmarks given via the SessionConfig.Builder.withBookmarks(...) method are modified afterwards. The driver does not expect this. Do you manage bookmarks explicitly?

[injectives]

I think it would make sense for the SessionConfig.Builder.withBookmarks(...) methods to actually make a copy since later modifications are not expected/supported. However, it would be best to find out what has led to this issue.
#1465

[seabamirum]

I use the ReactiveNeo4jClient from spring-data-neo4j-7.1.2 to execute queries. Perhaps some code in that project is modifying the bookmarks. There is an updateBookmarks call in the interface Neo4jBookmarkManager:

https://github.com/spring-projects/spring-data-neo4j/blob/b3d30af4d2ff1e714898fb8b59dcdd4b4e85ade9/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jBookmarkManager.java#L35

[injectives]

Opened a suggested fix: spring-projects/spring-data-neo4j#2769

[injectives]

This should be fixed now.

[seabamirum]

Unfortunately with version 5.11.0 I have the same problem, now in the SessionConfig class. Perhaps best to wait for the fix you pushed to the ReactiveNeo4jClient, at which point the copy of the bookmarks here should be irrelevant.

I feel bad reporting this the day of the release, but I had trouble building the pre-release version for an adequate test.

java.util.ConcurrentModificationException: null
	at java.base/java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1720) ~[na:na]

Original Stack Trace:
		at java.base/java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1720) ~[na:na]
		at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[na:na]
		at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[na:na]
		at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:575) ~[na:na]
		at java.base/java.util.stream.AbstractPipeline.evaluateToArrayNode(AbstractPipeline.java:260) ~[na:na]
		at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:616) ~[na:na]
		at java.base/java.util.stream.ReferencePipeline.toArray(ReferencePipeline.java:622) ~[na:na]
		at java.base/java.util.stream.ReferencePipeline.toList(ReferencePipeline.java:627) ~[na:na]
		at org.neo4j.driver.SessionConfig$Builder.withBookmarks(SessionConfig.java:265) ~[neo4j-java-driver-5.11.0.jar:5.11.0-ea56edc8c126b032c4a7dd8c4c571ec35419ca44]
		at org.springframework.data.neo4j.core.transaction.Neo4jTransactionUtils.sessionConfig(Neo4jTransactionUtils.java:72) ~[spring-data-neo4j-7.1.2.jar:7.1.2]
		at org.springframework.data.neo4j.core.DefaultReactiveNeo4jClient.lambda$getQueryRunner$0(DefaultReactiveNeo4jClient.java:94) ~[spring-data-neo4j-7.1.2.jar:7.1.2]

[injectives]

@seabamirum, thanks for checking. 👍

The driver update was not meant to eliminate the problem, it was just a general improvement in the driver. You need to get the updated ReactiveNeo4jClient and it should (hopefully) fix the issue you are experiencing.