Potential race condition in listener adding code

[o-l-o]

I'm pretty sure that there is potential race condition in the following code:

               Set<Topic> set = listenerTopics.get(listener);

		if (set == null) {
			set = new CopyOnWriteArraySet<>();
			listenerTopics.put(listener, set);
		}

The solution could be to replace it with something like this:

Set<Topic> set = listenerTopics.computeIfAbsent(listener, $ -> new CopyOnWriteArraySet<>());

spring-data-redis/src/main/java/org/springframework/data/redis/listener/RedisMessageListenerContainer.java

Line 649 in fc9f9d8

Set<Topic> set = listenerTopics.get(listener);

[jxblum]

Yes! Good find.

Any compound action (i.e. the Map get(:MessageListener), followed by a put(:MessageListener, :Set<Topic>), and then addAll(:Collection<Topic>)) is going to necessarily have possible race conditions if the affected area of code is 1) accessed by multiple Threads and 2) is not properly guarded by a lock, even if the underlying data structure(s) (Map & Set) are Thread-safe, which in this case, they are (a ConcurrentHashMap here and CopyOnWriteArraySet here).

Given 1) the Javadoc on the RedisMessageListenerContainer states the addMessageListener(..) method can be called concurrently without external synchronization, and 2) the addMessageListener(..) method calls the private addListener(..) method, then I'd say the code in question has race conditions.

[rotilho-bv]

I may also be affected by a race condition in RedisMessageListenerContainer. It doesn't happen while debugging the shutdown hook, but it happens every single time I don't. The thread dump makes me believe the error is caused by a race condition during re-registration during shutdown due to messages being sent.

Edit: removing message listener during shutdown workaround the problem.