Skip to content

Commit c98f758

Browse files
ArtemGamovolegz
authored andcommitted
GH-3116. Fix memory leak in StreamBridge on RefreshRemoteApplicationEvent #3116
Signed-off-by: gamovartem <art95081@rambler.ru> Resolves #3174
1 parent 3dae351 commit c98f758

2 files changed

Lines changed: 182 additions & 0 deletions

File tree

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
/*
2+
* Copyright 2023-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.cloud.stream.binder.kafka;
18+
19+
import java.lang.reflect.Field;
20+
import java.lang.reflect.Method;
21+
import java.util.Map;
22+
23+
import org.junit.jupiter.api.Test;
24+
25+
import org.springframework.boot.WebApplicationType;
26+
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
27+
import org.springframework.boot.builder.SpringApplicationBuilder;
28+
import org.springframework.cloud.stream.function.StreamBridge;
29+
import org.springframework.context.ConfigurableApplicationContext;
30+
import org.springframework.context.annotation.Configuration;
31+
import org.springframework.kafka.test.context.EmbeddedKafka;
32+
import org.springframework.messaging.MessageChannel;
33+
import org.springframework.test.annotation.DirtiesContext;
34+
35+
import static org.assertj.core.api.Assertions.assertThat;
36+
37+
/**
38+
* Test demonstrates that closeChannelsGracefully() closes producers
39+
*/
40+
@EmbeddedKafka
41+
@DirtiesContext
42+
class StreamBridgeCloseChannelsTests {
43+
44+
@Test
45+
void testCloseChannelsGracefullyClosesProducersButKeepsCache() throws Exception {
46+
try (ConfigurableApplicationContext context = new SpringApplicationBuilder(TestConfig.class)
47+
.web(WebApplicationType.NONE)
48+
.run(
49+
"--server.port=0",
50+
"--spring.jmx.enabled=false",
51+
"--spring.cloud.stream.kafka.binder.brokers=${spring.embedded.kafka.brokers}"
52+
)) {
53+
54+
StreamBridge streamBridge = context.getBean(StreamBridge.class);
55+
56+
// 1. Get the method
57+
Method closeMethod = StreamBridge.class.getDeclaredMethod("closeChannelsGracefully");
58+
closeMethod.setAccessible(true);
59+
60+
// 2. Get the cache
61+
Field cacheField = StreamBridge.class.getDeclaredField("channelCache");
62+
cacheField.setAccessible(true);
63+
Map<String, MessageChannel> cache = (Map<String, MessageChannel>) cacheField.get(streamBridge);
64+
65+
streamBridge.send("test-topic", "message-1");
66+
assertThat(cache).hasSize(1);
67+
MessageChannel originalChannel = cache.get("test-topic");
68+
69+
closeMethod.invoke(streamBridge);
70+
71+
assertThat(cache).hasSize(1);
72+
MessageChannel channelAfterClose = cache.get("test-topic");
73+
74+
assertThat(channelAfterClose).isSameAs(originalChannel);
75+
76+
boolean sent = streamBridge.send("test-topic", "message-2");
77+
assertThat(sent).isTrue();
78+
79+
MessageChannel channelAfterResend = cache.get("test-topic");
80+
assertThat(channelAfterResend).isSameAs(originalChannel);
81+
82+
streamBridge.send("test-topic-2", "message-3");
83+
assertThat(cache).hasSize(2); // Two channels
84+
85+
closeMethod.invoke(streamBridge);
86+
87+
assertThat(cache).hasSize(2);
88+
assertThat(cache).containsKeys("test-topic", "test-topic-2");
89+
}
90+
}
91+
92+
@Test
93+
void testOnApplicationEventClearsCache() throws Exception {
94+
try (ConfigurableApplicationContext context = new SpringApplicationBuilder(TestConfig.class)
95+
.web(WebApplicationType.NONE)
96+
.run(
97+
"--server.port=0",
98+
"--spring.jmx.enabled=false",
99+
"--spring.cloud.stream.kafka.binder.brokers=${spring.embedded.kafka.brokers}"
100+
)) {
101+
102+
StreamBridge streamBridge = context.getBean(StreamBridge.class);
103+
104+
// Get the cache
105+
Field cacheField = StreamBridge.class.getDeclaredField("channelCache");
106+
cacheField.setAccessible(true);
107+
Map<String, MessageChannel> cache = (Map<String, MessageChannel>) cacheField.get(streamBridge);
108+
109+
// Create channels
110+
streamBridge.send("topic-1", "data");
111+
streamBridge.send("topic-2", "data");
112+
streamBridge.send("topic-3", "data");
113+
114+
assertThat(cache).hasSize(3);
115+
116+
// Simulate what onApplicationEvent does
117+
// 1. Call closeChannelsGracefully()
118+
Method closeMethod = StreamBridge.class.getDeclaredMethod("closeChannelsGracefully");
119+
closeMethod.setAccessible(true);
120+
closeMethod.invoke(streamBridge);
121+
122+
// 2. Clear cache (what onApplicationEvent does after)
123+
cache.clear();
124+
assertThat(cache).isEmpty();
125+
126+
// Can create new channels
127+
streamBridge.send("new-topic", "data");
128+
assertThat(cache).hasSize(1);
129+
}
130+
}
131+
132+
@Test
133+
void testTheActualProblemAndSolution() throws Exception {
134+
try (ConfigurableApplicationContext context = new SpringApplicationBuilder(TestConfig.class)
135+
.web(WebApplicationType.NONE)
136+
.run(
137+
"--server.port=0",
138+
"--spring.jmx.enabled=false",
139+
"--spring.cloud.stream.kafka.binder.brokers=${spring.embedded.kafka.brokers}"
140+
)) {
141+
142+
StreamBridge streamBridge = context.getBean(StreamBridge.class);
143+
Field cacheField = StreamBridge.class.getDeclaredField("channelCache");
144+
cacheField.setAccessible(true);
145+
Map<String, MessageChannel> cache = (Map<String, MessageChannel>) cacheField.get(streamBridge);
146+
147+
// Example first usage
148+
streamBridge.send("daily-metrics", "data");
149+
streamBridge.send("user-events", "data");
150+
assertThat(cache).hasSize(2);
151+
152+
// Example second usage
153+
streamBridge.send("payment-events", "data");
154+
assertThat(cache).hasSize(3);
155+
}
156+
}
157+
158+
@EnableAutoConfiguration
159+
@Configuration
160+
public static class TestConfig {
161+
}
162+
}

core/spring-cloud-stream/src/main/java/org/springframework/cloud/stream/function/StreamBridge.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,10 +369,30 @@ public void setAsync(boolean async) {
369369
public void onApplicationEvent(ApplicationEvent event) {
370370
// we need to do it by String to avoid cloud-bus and context dependencies
371371
if (event.getClass().getName().equals("org.springframework.cloud.bus.event.RefreshRemoteApplicationEvent")) {
372+
closeChannelsGracefully();
372373
this.channelCache.clear();
373374
}
374375
}
375376

377+
private void closeChannelsGracefully() {
378+
this.channelCache.values().forEach(channel -> {
379+
try {
380+
// First try to unbind producers (existing cleanup logic)
381+
if (channel instanceof AbstractMessageChannel) {
382+
String channelName = ((AbstractMessageChannel) channel).getComponentName();
383+
if (channelName != null) {
384+
this.bindingService.unbindProducers(channelName);
385+
}
386+
}
387+
} catch (Exception e) {
388+
// Log but don't throw to ensure all channels get cleaned up
389+
if (logger.isDebugEnabled()) {
390+
logger.debug("Error while unbinding channel: " + channel, e);
391+
}
392+
}
393+
});
394+
}
395+
376396
private static final class ContextPropagationHelper {
377397
static ExecutorService wrap(ExecutorService executorService) {
378398
return ContextExecutorService.wrap(executorService, () -> ContextSnapshotFactory.builder().build().captureAll());

0 commit comments

Comments
 (0)