spring-projects / spring-data-redis

Provides support to increase developer productivity in Java when using Redis, a key-value store. Uses familiar Spring concepts such as a template classes for core API usage and lightweight repository style data access.
https://spring.io/projects/spring-data-redis/
Apache License 2.0
1.77k stars 1.17k forks source link

Correctly handle `null` listener in `RedisMessageListenerContainer.remove(…)` #3009

Closed sujl95 closed 1 month ago

sujl95 commented 1 month ago

Hello,

While using the remove API, I encountered a NullPointerException (NPE) when the listener parameter was null. This PR addresses that issue by updating the remove method in RedisMessageListenerContainer.

The proposed changes ensure that when the listener is null, all listeners associated with the specified topic are removed. Additionally, the listenerTopics and mapping collections are cleaned up properly to prevent NPEs, making the listener removal process more robust.

If this contribution does not meet any of the project’s contribution requirements, please feel free to let me know, and I’ll be happy to make any necessary adjustments.

Summary:

This PR improves the remove() method in RedisMessageListenerContainer by ensuring that it gracefully handles null listeners. When the listener parameter is null, the method will now remove all associated listeners for the specified topic. The method has been updated to prevent NullPointerExceptions (NPEs) and ensure proper cleanup of the listenerTopics and mapping collections.

Description of the problem: In the current implementation of RedisMessageListenerContainer.remove(), a potential NullPointerException (NPE) can occur when the listener parameter is null. This leads to unexpected behavior when trying to remove listeners for a specific topic, especially when no listeners are associated with the topic, or the listener is null.

Without proper handling, the method can attempt to remove a null listener or update the listenerTopics and mapping in a way that results in an NPE, disrupting the listener removal process.

Proposed solution: This PR updates the remove() method in RedisMessageListenerContainer to correctly handle cases where the listener parameter is null. Specifically:

These changes ensure that the listener removal process is more robust and handles edge cases involving null listeners and empty mappings more gracefully.

Changes:

How to reproduce the issue: The issue can be triggered by calling the remove() method with a null listener while there are no listeners associated with the topic in listenerTopics. This would result in an NPE when trying to update the collections.

Testing:

mp911de commented 1 month ago

Care to attach a stack trace of your NPE?

sujl95 commented 1 month ago

@mp911de

Hello,

As requested, I'm sharing the stack trace where the NullPointerException (NPE) occurs when using the remove() method in RedisMessageListenerContainer. The issue happens when the listener parameter is null, causing the hashCode() method to be invoked on a null key. Please find the attached stack trace screenshot for further details.

If you need any additional information, feel free to let me know. Thank you!

스크린샷 2024-10-01 오전 12 17 29
java.lang.NullPointerException: Cannot invoke "Object.hashCode()" because "key" is null
    at java.base/java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936)
    at org.springframework.data.redis.listener.RedisMessageListenerContainer.remove(RedisMessageListenerContainer.java:774)
    at org.springframework.data.redis.listener.RedisMessageListenerContainer.removeListener(RedisMessageListenerContainer.java:731)
    at org.springframework.data.redis.listener.RedisMessageListenerContainer.removeMessageListener(RedisMessageListenerContainer.java:562)
    at org.springframework.data.redis.listener.RedisMessageListenerContainer.removeMessageListener(RedisMessageListenerContainer.java:576)
mp911de commented 1 month ago

Thank you for your contribution. That's merged, polished, and backported now.