Skip to content

Commit 1c16075

Browse files
committed
Fix race condition in the AbstractMessageListenerContainer lifecycle
The `stop()` and `childStopped()` in the `ConcurrentMessageListenerContainer` use the same `lifecycleLock`, however the last one is called from the thread of listener consumer in the child application context. * Fix `AbstractMessageListenerContainer.stop(wait)` logic to release the `lifecycleLock` before going to the `latch.await()`. This still blocks the `stop()` call, but allows the other lifecycle conditions to be fulfilled, even from different threads
1 parent c6cf896 commit 1c16075

File tree

1 file changed

+26
-19
lines changed

1 file changed

+26
-19
lines changed

spring-kafka/src/main/java/org/springframework/kafka/listener/AbstractMessageListenerContainer.java

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
*/
7676
public abstract class AbstractMessageListenerContainer<K, V>
7777
implements GenericMessageListenerContainer<K, V>, BeanNameAware, ApplicationEventPublisherAware,
78-
ApplicationContextAware {
78+
ApplicationContextAware {
7979

8080
/**
8181
* The default {@link org.springframework.context.SmartLifecycle} phase for listener
@@ -143,7 +143,6 @@ public abstract class AbstractMessageListenerContainer<K, V>
143143
@Nullable
144144
private KafkaAdmin kafkaAdmin;
145145

146-
147146
/**
148147
* Construct an instance with the provided factory and properties.
149148
* @param consumerFactory the factory.
@@ -609,28 +608,36 @@ public final void stop() {
609608
* @since 2.3.8
610609
*/
611610
public final void stop(boolean wait) {
612-
this.lifecycleLock.lock();
613-
try {
614-
if (isRunning()) {
615-
if (wait) {
616-
final CountDownLatch latch = new CountDownLatch(1);
611+
if (isRunning()) {
612+
if (wait) {
613+
final CountDownLatch latch = new CountDownLatch(1);
614+
this.lifecycleLock.lock();
615+
try {
616+
617617
doStop(latch::countDown);
618-
try {
619-
latch.await(this.containerProperties.getShutdownTimeout(), TimeUnit.MILLISECONDS); // NOSONAR
620-
publishContainerStoppedEvent();
621-
}
622-
catch (@SuppressWarnings("unused") InterruptedException e) {
623-
Thread.currentThread().interrupt();
624-
}
625618
}
626-
else {
619+
finally {
620+
this.lifecycleLock.unlock();
621+
}
622+
try {
623+
latch.await(this.containerProperties.getShutdownTimeout(), TimeUnit.MILLISECONDS); // NOSONAR
624+
publishContainerStoppedEvent();
625+
}
626+
catch (@SuppressWarnings("unused") InterruptedException e) {
627+
Thread.currentThread().interrupt();
628+
}
629+
}
630+
else {
631+
this.lifecycleLock.lock();
632+
try {
627633
doStop(this::publishContainerStoppedEvent);
628634
}
635+
finally {
636+
this.lifecycleLock.unlock();
637+
}
638+
629639
}
630640
}
631-
finally {
632-
this.lifecycleLock.unlock();
633-
}
634641
}
635642

636643
@Override
@@ -706,7 +713,7 @@ public void onPartitionsAssigned(Collection<TopicPartition> partitions) {
706713
@Override
707714
public void onPartitionsLost(Collection<TopicPartition> partitions) {
708715
AbstractMessageListenerContainer.this.logger.info(() ->
709-
getGroupId() + ": partitions lost: " + partitions);
716+
getGroupId() + ": partitions lost: " + partitions);
710717
}
711718

712719
};

0 commit comments

Comments
 (0)