Skip to content

Commit b477277

Browse files
authored
GH-3701: Fix Possible TCP Memory Leak
Resolves #3701 Ensure `TcpSender.removeDeadConnection` is always called, for example when intercepted and closed via `factory.closeConnectionId` or when closed connections are harvested from the `connections` map. **Cherry-pick to 5.4.x, 5.3.x**
1 parent 9ff2707 commit b477277

File tree

2 files changed

+113
-2
lines changed

2 files changed

+113
-2
lines changed

Diff for: spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractConnectionFactory.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -566,6 +566,7 @@ public void stop() {
566566
TcpConnection connection = iterator.next().getValue();
567567
connection.close();
568568
iterator.remove();
569+
getSenders().forEach(sender -> sender.removeDeadConnection(connection));
569570
}
570571
}
571572
synchronized (this.lifecycleMonitor) {
@@ -865,6 +866,7 @@ private List<String> removeClosedConnectionsAndReturnOpenConnectionIds() {
865866
TcpConnectionSupport connection = entry.getValue();
866867
if (!connection.isOpen()) {
867868
iterator.remove();
869+
getSenders().forEach(sender -> sender.removeDeadConnection(connection));
868870
logger.debug(() -> getComponentName() + ": Removed closed connection: " +
869871
connection.getConnectionId());
870872
}
@@ -937,11 +939,12 @@ public boolean closeConnection(String connectionId) {
937939
// closed connections are removed from #connections in #harvestClosedConnections()
938940
synchronized (this.connections) {
939941
boolean closed = false;
940-
TcpConnectionSupport connection = this.connections.get(connectionId);
942+
TcpConnectionSupport connection = this.connections.remove(connectionId);
941943
if (connection != null) {
942944
try {
943945
connection.close();
944946
closed = true;
947+
getSenders().forEach(sender -> sender.removeDeadConnection(connection));
945948
}
946949
catch (Exception ex) {
947950
logger.debug(ex, () -> "Failed to close connection " + connectionId);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Copyright 2022 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.integration.ip.tcp.connection;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
21+
import java.util.concurrent.CountDownLatch;
22+
import java.util.concurrent.TimeUnit;
23+
24+
import org.junit.jupiter.api.Test;
25+
26+
/**
27+
* @author Gary Russell
28+
* @since 5.3.10
29+
*
30+
*/
31+
public class TcpSenderTests {
32+
33+
@Test
34+
void senderCalledForDeadConnectionClientNet() throws InterruptedException {
35+
CountDownLatch latch = new CountDownLatch(1);
36+
TcpNetServerConnectionFactory server = new TcpNetServerConnectionFactory(0);
37+
server.registerListener(msg -> false);
38+
server.afterPropertiesSet();
39+
server.setApplicationEventPublisher(event -> {
40+
if (event instanceof TcpConnectionServerListeningEvent) {
41+
latch.countDown();
42+
}
43+
});
44+
server.start();
45+
assertThat(latch.await(10, TimeUnit.SECONDS)).isTrue();
46+
TcpNetClientConnectionFactory client = new TcpNetClientConnectionFactory("localhost", server.getPort());
47+
senderCalledForDeadConnectionClient(client);
48+
server.stop();
49+
}
50+
51+
@Test
52+
void senderCalledForDeadConnectionClientNio() throws InterruptedException {
53+
CountDownLatch latch = new CountDownLatch(1);
54+
TcpNetServerConnectionFactory server = new TcpNetServerConnectionFactory(0);
55+
server.registerListener(msg -> false);
56+
server.afterPropertiesSet();
57+
server.setApplicationEventPublisher(event -> {
58+
if (event instanceof TcpConnectionServerListeningEvent) {
59+
latch.countDown();
60+
}
61+
});
62+
server.start();
63+
assertThat(latch.await(10, TimeUnit.SECONDS)).isTrue();
64+
TcpNioClientConnectionFactory client = new TcpNioClientConnectionFactory("localhost", server.getPort());
65+
senderCalledForDeadConnectionClient(client);
66+
server.stop();
67+
}
68+
69+
private void senderCalledForDeadConnectionClient(AbstractClientConnectionFactory client) throws InterruptedException {
70+
CountDownLatch adds = new CountDownLatch(2);
71+
CountDownLatch removes = new CountDownLatch(2);
72+
TcpConnectionInterceptorFactoryChain chain = new TcpConnectionInterceptorFactoryChain();
73+
chain.setInterceptor(new HelloWorldInterceptorFactory() {
74+
75+
@Override
76+
public TcpConnectionInterceptorSupport getInterceptor() {
77+
return new TcpConnectionInterceptorSupport() {
78+
};
79+
}
80+
81+
});
82+
client.setInterceptorFactoryChain(chain);
83+
client.registerSender(new TcpSender() {
84+
85+
@Override
86+
public void addNewConnection(TcpConnection connection) {
87+
adds.countDown();
88+
}
89+
90+
@Override
91+
public void removeDeadConnection(TcpConnection connection) {
92+
removes.countDown();
93+
}
94+
95+
});
96+
client.setSingleUse(true);
97+
client.afterPropertiesSet();
98+
client.start();
99+
TcpConnectionSupport conn = client.getConnection();
100+
conn.close();
101+
conn = client.getConnection();
102+
assertThat(adds.await(10, TimeUnit.SECONDS)).isTrue();
103+
conn.close();
104+
client.stop();
105+
assertThat(removes.await(10, TimeUnit.SECONDS)).isTrue();
106+
}
107+
108+
}

0 commit comments

Comments
 (0)