Skip to content

Commit ecfaf27

Browse files
authored
Fix PgEncoder to fully close the connection on idle timeout (#1760) (#1761)
See #1755 The `PgEncoder` close override sends the Terminate message but was not propagating the close, so the channel never fully closed when `readIdleTimeout` fired on an unresponsive connection. Propagate `ctx.close(promise)` when `closeSent` is false; otherwise wire the promise to `ch.closeFuture()` to let the normal `shutdownOutput` cycle complete without cutting off in-flight server responses. Add `testReadIdleTimeoutClosesConnection` using a Vert.x `NetServer` with `ALLOW_HALF_CLOSURE` to simulate an unresponsive server that doesn't send FIN back. Fix `testMaximumParametersExceeded` to use `asyncAssertFailure` so the VertxUnit runner waits for the query result. Without this, the runner tears down the `Vertx` instance immediately after the test method returns, closing the PG connection before the server response arrives, which changes the failure reason from a parameter count error to a connection lost error. Assisted-By: Claude Opus 4.6 (cherry picked from commit faf9310) Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
1 parent 4756d09 commit ecfaf27

3 files changed

Lines changed: 62 additions & 6 deletions

File tree

‎vertx-pg-client/src/main/java/io/vertx/pgclient/impl/codec/PgEncoder.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@
2323
import io.netty.channel.ChannelOutboundHandlerAdapter;
2424
import io.netty.channel.ChannelPromise;
2525
import io.netty.channel.socket.SocketChannel;
26-
import io.vertx.sqlclient.Tuple;
26+
import io.netty.util.concurrent.PromiseNotifier;
2727
import io.vertx.pgclient.impl.util.Util;
28+
import io.vertx.sqlclient.Tuple;
2829
import io.vertx.sqlclient.impl.HexSequence;
2930
import io.vertx.sqlclient.internal.RowDescriptorBase;
3031
import io.vertx.sqlclient.spi.protocol.CloseConnectionCommand;
3132

32-
import java.util.*;
33+
import java.util.ArrayList;
34+
import java.util.Map;
3335

3436
import static io.vertx.pgclient.impl.util.Util.writeCString;
3537
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -98,6 +100,9 @@ public void close(ChannelHandlerContext ctx, ChannelPromise promise) throws Exce
98100
CloseConnectionCommand cmd = CloseConnectionCommand.INSTANCE;
99101
PgCommandMessage<?, ?> codec = PgCommandMessage.wrap(cmd);
100102
codec.encode(this);
103+
ctx.close(promise);
104+
} else {
105+
ctx.channel().closeFuture().addListener(new PromiseNotifier<>(promise));
101106
}
102107
}
103108

‎vertx-pg-client/src/test/java/io/vertx/tests/pgclient/CloseConnectionTest.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,26 @@
11
package io.vertx.tests.pgclient;
22

33
import io.netty.buffer.ByteBufUtil;
4+
import io.netty.channel.ChannelOption;
45
import io.vertx.core.Vertx;
56
import io.vertx.core.buffer.Buffer;
7+
import io.vertx.core.internal.net.NetSocketInternal;
68
import io.vertx.ext.unit.Async;
79
import io.vertx.ext.unit.TestContext;
810
import io.vertx.pgclient.PgBuilder;
11+
import io.vertx.pgclient.PgConnectOptions;
912
import io.vertx.pgclient.PgConnection;
13+
import io.vertx.pgclient.SslMode;
1014
import io.vertx.sqlclient.Pool;
1115
import io.vertx.sqlclient.PoolOptions;
1216
import io.vertx.tests.sqlclient.ProxyServer;
1317
import org.junit.After;
1418
import org.junit.Before;
1519
import org.junit.Test;
1620

21+
import java.nio.charset.StandardCharsets;
22+
import java.util.concurrent.TimeUnit;
23+
1724
public class CloseConnectionTest extends PgTestBase {
1825

1926
protected Vertx vertx;
@@ -74,6 +81,51 @@ private void testCloseConnection(TestContext ctx, Runnable test) {
7481
}));
7582
}
7683

84+
@Test
85+
public void testReadIdleTimeoutClosesConnection(TestContext ctx) {
86+
Async async = ctx.async();
87+
vertx.createNetServer().connectHandler(socket -> {
88+
// Enable half-close so the server keeps the connection open after receiving FIN,
89+
// otherwise Vert.x auto-closes and sends FIN back, masking the bug
90+
((NetSocketInternal) socket).channelHandlerContext().channel().config()
91+
.setOption(ChannelOption.ALLOW_HALF_CLOSURE, true);
92+
socket.handler(data -> {
93+
// Received startup message, send PG handshake response then stay silent
94+
Buffer response = Buffer.buffer()
95+
.appendByte((byte) 'R').appendInt(8).appendInt(0)
96+
.appendByte((byte) 'S').appendInt(25)
97+
.appendBytes("client_encoding\0UTF8\0".getBytes(StandardCharsets.UTF_8))
98+
.appendByte((byte) 'K').appendInt(12).appendInt(1).appendInt(0)
99+
.appendByte((byte) 'Z').appendInt(5).appendByte((byte) 'I');
100+
socket.write(response);
101+
socket.handler(null);
102+
});
103+
}).listen(0, "localhost").onComplete(ctx.asyncAssertSuccess(server -> {
104+
PgConnectOptions connectOptions = new PgConnectOptions()
105+
.setHost("localhost")
106+
.setPort(server.actualPort())
107+
.setUser("test")
108+
.setPassword("test")
109+
.setDatabase("test")
110+
.setSslMode(SslMode.DISABLE);
111+
Pool pool = PgBuilder.pool()
112+
.connectingTo(connectOptions)
113+
.with(new PoolOptions().setMaxSize(1))
114+
.with(new io.vertx.core.net.NetClientOptions()
115+
.setReadIdleTimeout(200)
116+
.setIdleTimeoutUnit(TimeUnit.MILLISECONDS))
117+
.using(vertx)
118+
.build();
119+
pool.getConnection().onComplete(ctx.asyncAssertSuccess(conn ->
120+
conn.closeHandler(v -> {
121+
pool.close();
122+
server.close();
123+
async.complete();
124+
})
125+
));
126+
}));
127+
}
128+
77129
@Test
78130
public void testTransactionInProgressShouldFail(TestContext ctx) {
79131
ProxyServer proxy = ProxyServer.create(vertx, options.getPort(), options.getHost());

‎vertx-pg-client/src/test/java/io/vertx/tests/pgclient/tck/PgPreparedQueryTestBase.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,9 @@ public void testMaximumParametersExceeded(TestContext ctx) {
5252
connect(ctx.asyncAssertSuccess(conn -> {
5353
conn.preparedQuery(query)
5454
.execute(tuple)
55-
.onComplete(ar -> {
56-
ctx.assertTrue(ar.failed());
57-
ctx.assertTrue(ar.cause().getMessage().contains("exceeds the maximum of 65535"));
58-
});
55+
.onComplete(ctx.asyncAssertFailure(t -> {
56+
ctx.assertTrue(t.getMessage().contains("exceeds the maximum of 65535"));
57+
}));
5958
}));
6059
}
6160
}

0 commit comments

Comments
 (0)