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.

Sign up for GitHub

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

Jump to bottom

KafkaMessageListenerContainer.getAssignedPartitions is not safe #3726

Closed
tbarabanov opened this issue Jan 29, 2025 · 1 comment · Fixed by #3728
Closed

KafkaMessageListenerContainer.getAssignedPartitions is not safe #3726

tbarabanov opened this issue Jan 29, 2025 · 1 comment · Fixed by #3728
Labels
for: backport-to-3.2.x status: backported type: bug
Milestone
3.3.3

Comments

@tbarabanov
Copy link
Contributor

In what version(s) of Spring for Apache Kafka are you seeing this issue?

3.2.1

Describe the bug

KafkaMessageListenerContainer.getAssignedPartitions() is not safe due to the fact that different threads can iterate/modify any of the fields partitionsListenerConsumer.definedPartitions, partitionsListenerConsumer.assignedPartitions simultaneously, but collection types of these fields are not designed for such scenarios. Thus at least ConcurrentModificationException can be thrown.

Expected behavior

KafkaMessageListenerContainer.getAssignedPartitions() method is public and thus expected to be safe.
@artembilan artembilan added this to the 3.3.3 milestone Jan 29, 2025
@artembilan
Copy link
Member

More info in StackOverflow question: https://stackoverflow.com/questions/79383650/is-kafkamessagelistenercontainer-getassignedpartitions-thread-safe.

@tbarabanov ,

feel free to contribute the fix: https://github.com/spring-projects/spring-kafka/blob/main/CONTRIBUTING.adoc!

tbarabanov added a commit to tbarabanov/spring-kafka that referenced this issue Jan 30, 2025
@tbarabanov
spring-projectsGH-3726: Avoid ConcurrentModificationException in Kafk…
f9672a9 
…aMessageListenerContainer

Fixes spring-projectsGH-3726 (spring-projects#3726)
@tbarabanov tbarabanov mentioned this issue Jan 30, 2025
Merged
tbarabanov added a commit to tbarabanov/spring-kafka that referenced this issue Jan 30, 2025
@tbarabanov
spring-projectsGH-3726: Avoid ConcurrentModificationException in Kafk…
d3a4ae7 
…aMessageListenerContainer

Fixes spring-projectsGH-3726 (spring-projects#3726)

Signed-off-by: Tim Barabanov <t.barabanov@gmail.com>
tbarabanov added a commit to tbarabanov/spring-kafka that referenced this issue Jan 30, 2025
@tbarabanov
spring-projectsGH-3726: Avoid ConcurrentModificationException in Kafk…
ba2f592 
…aMessageListenerContainer

Fixes spring-projectsGH-3726 (spring-projects#3726)

Signed-off-by: Tim Barabanov <t.barabanov@gmail.com>
artembilan pushed a commit that referenced this issue Jan 30, 2025
@tbarabanov
GH-3726: Fix KafkaMessageListenerContainer for ConcurrentModification…

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
Loading
Loading status checks…
b772ebd 
…Exception

Fixes: #3726
Issue link: #3726

`KafkaMessageListenerContainer.getAssignedPartitions()` is not safe due to the fact that 
different threads can iterate/modify any of the fields `partitionsListenerConsumer.definedPartitions` or `partitionsListenerConsumer.assignedPartitions` simultaneously, 
but collection types of these fields are not designed for such scenarios. 
Thus at least `ConcurrentModificationException` can be thrown.

* Wrap `partitionsListenerConsumer.definedPartitions` and `partitionsListenerConsumer.assignedPartitions`  into `Collections.synchronizedSet()`

Signed-off-by: Tim Barabanov <t.barabanov@gmail.com>

[artem.bilan@broadcom.com Fix commit message]

**Auto-cherry-pick to `3.2.x`**

Signed-off-by: Artem Bilan <artem.bilan@broadcom.com>
artembilan pushed a commit that referenced this issue Jan 30, 2025
@tbarabanov @artembilan
GH-3726: Fix KafkaMessageListenerContainer for ConcurrentModification…

Verified

This commit was signed with the committer’s verified signature.
artembilan Artem Bilan
SSH Key Fingerprint: kI2VT3GW4Bvxdz5hbsh73B1IW6fHrohOOCcA+P+na6I
Verified
Learn about vigilant mode
Loading
Loading status checks…
bbaecbc 
…Exception

Fixes: #3726
Issue link: #3726

`KafkaMessageListenerContainer.getAssignedPartitions()` is not safe due to the fact that
different threads can iterate/modify any of the fields `partitionsListenerConsumer.definedPartitions` or `partitionsListenerConsumer.assignedPartitions` simultaneously,
but collection types of these fields are not designed for such scenarios.
Thus at least `ConcurrentModificationException` can be thrown.

* Wrap `partitionsListenerConsumer.definedPartitions` and `partitionsListenerConsumer.assignedPartitions`  into `Collections.synchronizedSet()`

Signed-off-by: Tim Barabanov <t.barabanov@gmail.com>

[artem.bilan@broadcom.com Fix commit message]

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java

Signed-off-by: Artem Bilan <artem.bilan@broadcom.com>
@spring-builds spring-builds mentioned this issue Jan 30, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
for: backport-to-3.2.x status: backported type: bug
Projects
None yet
Milestone
3.3.3
3 participants
@artembilan @spring-builds @tbarabanov