From 162ef4a60e31daeaf64f043063fed965ee4bc69b Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Fri, 6 Dec 2024 09:43:12 -0500 Subject: [PATCH] Attempt to fix race condition in the `CachingConnectionFactory` There is a logic in the `CachingConnectionFactory.destroy()` where we wait for `inFlightAsyncCloses`. However, it looks like we first close the main connection with all its channels. Therefore, we may end up with a `ConcurrentModificationException` when we iterate channels list for clean up, but in-flight requests still wait for their confirms, therefore they may come back to the cache meanwhile we are trying to clean up --- .../rabbit/connection/CachingConnectionFactory.java | 4 ++-- ...itTemplatePublisherCallbacksIntegration1Tests.java} | 4 ++-- ...itTemplatePublisherCallbacksIntegration2Tests.java} | 8 ++++---- ...itTemplatePublisherCallbacksIntegration3Tests.java} | 10 +++++----- 4 files changed, 13 insertions(+), 13 deletions(-) rename spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/{RabbitTemplatePublisherCallbacksIntegrationTests.java => RabbitTemplatePublisherCallbacksIntegration1Tests.java} (99%) rename spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/{RabbitTemplatePublisherCallbacksIntegrationTests2.java => RabbitTemplatePublisherCallbacksIntegration2Tests.java} (96%) rename spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/{RabbitTemplatePublisherCallbacksIntegrationTests3.java => RabbitTemplatePublisherCallbacksIntegration3Tests.java} (95%) diff --git a/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/connection/CachingConnectionFactory.java b/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/connection/CachingConnectionFactory.java index 0aade483..302b18a5 100644 --- a/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/connection/CachingConnectionFactory.java +++ b/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/connection/CachingConnectionFactory.java @@ -861,8 +861,6 @@ public class CachingConnectionFactory extends AbstractConnectionFactory */ @Override public final void destroy() { - super.destroy(); - resetConnection(); if (getContextStopped()) { this.stopped = true; this.connectionLock.lock(); @@ -890,6 +888,8 @@ public class CachingConnectionFactory extends AbstractConnectionFactory this.connectionLock.unlock(); } } + super.destroy(); + resetConnection(); } /** diff --git a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegrationTests.java b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegration1Tests.java similarity index 99% rename from spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegrationTests.java rename to spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegration1Tests.java index 1429b7f5..fef64706 100644 --- a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegrationTests.java +++ b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegration1Tests.java @@ -95,8 +95,8 @@ import com.rabbitmq.client.ConnectionFactory; * @since 1.1 * */ -@RabbitAvailable(queues = RabbitTemplatePublisherCallbacksIntegrationTests.ROUTE) -public class RabbitTemplatePublisherCallbacksIntegrationTests { +@RabbitAvailable(queues = RabbitTemplatePublisherCallbacksIntegration1Tests.ROUTE) +public class RabbitTemplatePublisherCallbacksIntegration1Tests { public static final String ROUTE = "test.queue.RabbitTemplatePublisherCallbacksIntegrationTests"; diff --git a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegrationTests2.java b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegration2Tests.java similarity index 96% rename from spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegrationTests2.java rename to spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegration2Tests.java index 450c1566..06837b98 100644 --- a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegrationTests2.java +++ b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegration2Tests.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2022 the original author or authors. + * Copyright 2016-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,9 +41,9 @@ import com.rabbitmq.client.Envelope; * @since 1.6 * */ -@RabbitAvailable(queues = { RabbitTemplatePublisherCallbacksIntegrationTests2.ROUTE, - RabbitTemplatePublisherCallbacksIntegrationTests2.ROUTE2 }) -public class RabbitTemplatePublisherCallbacksIntegrationTests2 { +@RabbitAvailable(queues = { RabbitTemplatePublisherCallbacksIntegration2Tests.ROUTE, + RabbitTemplatePublisherCallbacksIntegration2Tests.ROUTE2 }) +public class RabbitTemplatePublisherCallbacksIntegration2Tests { public static final String ROUTE = "test.queue.RabbitTemplatePublisherCallbacksIntegrationTests2"; diff --git a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegrationTests3.java b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegration3Tests.java similarity index 95% rename from spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegrationTests3.java rename to spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegration3Tests.java index 8e7fcef7..d9f1dddd 100644 --- a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegrationTests3.java +++ b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitTemplatePublisherCallbacksIntegration3Tests.java @@ -1,5 +1,5 @@ /* - * Copyright 2018-2021 the original author or authors. + * Copyright 2018-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -41,10 +41,10 @@ import com.rabbitmq.client.Channel; * @since 2.1 * */ -@RabbitAvailable(queues = { RabbitTemplatePublisherCallbacksIntegrationTests3.QUEUE1, - RabbitTemplatePublisherCallbacksIntegrationTests3.QUEUE2, - RabbitTemplatePublisherCallbacksIntegrationTests3.QUEUE3 }) -public class RabbitTemplatePublisherCallbacksIntegrationTests3 { +@RabbitAvailable(queues = { RabbitTemplatePublisherCallbacksIntegration3Tests.QUEUE1, + RabbitTemplatePublisherCallbacksIntegration3Tests.QUEUE2, + RabbitTemplatePublisherCallbacksIntegration3Tests.QUEUE3 }) +public class RabbitTemplatePublisherCallbacksIntegration3Tests { public static final String QUEUE1 = "synthetic.nack";