Skip to content

Commit f3bf835

Browse files
committed
Fix socket leak and wrong exception propagation on connection error
See #1672, #1674, #1675 When a connection handles a failure, the underlying Netty socket should be closed, avoiding leak of file descriptors and Netty resources. Additionally, inflight commands shouldn't fail with ClosedConnectionException but with the actual error. Some portions of this content were created with the assistance of Claude Code. Signed-off-by: Thomas Segismont <tsegismont@gmail.com>
1 parent 5fc500e commit f3bf835

4 files changed

Lines changed: 34 additions & 7 deletions

File tree

vertx-mssql-client/src/test/java/io/vertx/tests/mssqlclient/MSSQLQueriesTest.java

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@
4040
import java.util.stream.Collectors;
4141
import java.util.stream.Stream;
4242

43-
import static org.hamcrest.CoreMatchers.instanceOf;
44-
import static org.hamcrest.CoreMatchers.is;
43+
import static org.hamcrest.CoreMatchers.*;
4544
import static org.hamcrest.MatcherAssert.assertThat;
4645

4746
@RunWith(VertxUnitRunner.class)
@@ -206,4 +205,29 @@ public void testQuerySequences(TestContext ctx) {
206205
}));
207206
}));
208207
}
208+
209+
@Test
210+
public void testUnsupportedXmlTypeErrorPropagation(TestContext ctx) {
211+
Async async = ctx.async();
212+
connection
213+
.query("CREATE TABLE #TempXmlTest (id INT, xmlData XML)")
214+
.execute()
215+
.onComplete(ctx.asyncAssertSuccess(create -> {
216+
connection
217+
.query("SELECT * FROM #TempXmlTest")
218+
.execute()
219+
.onComplete(ctx.asyncAssertFailure(t -> {
220+
ctx.verify(v -> {
221+
assertThat(t, instanceOf(UnsupportedOperationException.class));
222+
assertThat(t.getMessage(), containsString("XML"));
223+
});
224+
connection
225+
.query("SELECT 1")
226+
.execute()
227+
.onComplete(ctx.asyncAssertFailure(t2 -> {
228+
connection.close().onComplete(ctx.asyncAssertSuccess(v -> async.complete()));
229+
}));
230+
}));
231+
}));
232+
}
209233
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
package io.vertx.tests.pgclient;
1313

14+
import com.ongres.scram.client.ChannelBindingException;
1415
import io.vertx.core.Vertx;
1516
import io.vertx.core.VertxOptions;
1617
import io.vertx.core.buffer.Buffer;
@@ -25,7 +26,6 @@
2526
import io.vertx.pgclient.PgConnection;
2627
import io.vertx.pgclient.SslMode;
2728
import io.vertx.pgclient.SslNegotiation;
28-
import io.vertx.sqlclient.ClosedConnectionException;
2929
import io.vertx.sqlclient.Tuple;
3030
import io.vertx.tests.pgclient.junit.ContainerPgRule;
3131
import org.junit.*;
@@ -287,8 +287,7 @@ public void testChannelBindingRequireWithoutSsl(TestContext ctx) {
287287
.setSslOptions(new ClientSSLOptions().setTrustAll(true));
288288

289289
PgConnection.connect(vertx, options).onComplete(ctx.asyncAssertFailure(err -> {
290-
// ctx.assertEquals("Channel bindins is required", err.getMessage());
291-
ctx.assertTrue(err instanceof ClosedConnectionException); // TODO: handle ChannelBindingException
290+
ctx.assertTrue(err instanceof ChannelBindingException);
292291
async.complete();
293292
}));
294293
}

vertx-pg-client/src/test/java/module-info.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
requires io.netty.buffer;
44
requires io.netty.transport;
5+
requires com.ongres.scram.client;
56
requires io.vertx.core;
67
requires io.vertx.sql.client;
78
requires io.vertx.sql.client.pg;

vertx-sql-client-codec/src/main/java/io/vertx/sqlclient/codec/SocketConnectionBase.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,8 @@ protected void handleException(Throwable t) {
421421
DecoderException err = (DecoderException) t;
422422
t = err.getCause();
423423
}
424+
socket.closeHandler(null);
425+
socket.close();
424426
handleClose(t);
425427
}
426428

@@ -441,11 +443,12 @@ protected void handleClose(Throwable t) {
441443
if (t != null) {
442444
reportException(t);
443445
}
446+
Throwable inflightCause = t != null ? t : ClosedConnectionException.INSTANCE;
444447
CommandMessage<?, ?> msg;
445448
while ((msg = inflights.poll()) != null) {
446-
fail(msg, ClosedConnectionException.INSTANCE);
449+
fail(msg, inflightCause);
447450
}
448-
Throwable cause = t == null ? VertxException.noStackTrace(PENDING_CMD_CONNECTION_CORRUPT_MSG) : new VertxException(PENDING_CMD_CONNECTION_CORRUPT_MSG, t);
451+
Throwable cause = t == null ? VertxException.noStackTrace(PENDING_CMD_CONNECTION_CORRUPT_MSG) : VertxException.noStackTrace(PENDING_CMD_CONNECTION_CORRUPT_MSG, t);
449452
CommandBase<?> cmd;
450453
while ((cmd = pending.poll()) != null) {
451454
CommandBase c = cmd;

0 commit comments

Comments
 (0)