Skip to content

Commit d211a02

Browse files
committed
Fail acquisition with the hook failure and release the lease when afterAcquire fails
When the afterAcquire hook failed during SqlConnectionPool#acquire, the acquisition was failed with the enclosing (null) failure instead of the hook failure, so callers observed a null cause. The lease was also neither recycled nor closed, leaking the pool slot: with enough hook failures the pool starved with no way to recover. Recycle the lease and fail the acquisition with the actual hook failure. Signed-off-by: andrei <andrei@n2.games>
1 parent 2848fdf commit d211a02

2 files changed

Lines changed: 165 additions & 2 deletions

File tree

vertx-sql-client/src/main/java/io/vertx/sqlclient/impl/pool/SqlConnectionPool.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,10 @@ public void complete(Lease<PooledConnection> lease, Throwable failure) {
279279
if (ar2.succeeded()) {
280280
handle(lease);
281281
} else {
282-
// Should we do some cleanup ?
283-
handler.fail(failure);
282+
// return the lease to the pool, otherwise its slot leaks forever
283+
lease.recycle();
284+
dequeueMetric(metric);
285+
handler.fail(ar2.cause());
284286
}
285287
});
286288
} else {
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
/*
2+
* Copyright (c) 2011-2026 Contributors to the Eclipse Foundation
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
7+
* which is available at https://www.apache.org/licenses/LICENSE-2.0.
8+
*
9+
* SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
10+
*/
11+
12+
package io.vertx.tests.sqlclient;
13+
14+
import io.vertx.core.Completable;
15+
import io.vertx.core.Context;
16+
import io.vertx.core.Future;
17+
import io.vertx.core.Vertx;
18+
import io.vertx.core.internal.ContextInternal;
19+
import io.vertx.core.internal.VertxInternal;
20+
import io.vertx.core.net.SocketAddress;
21+
import io.vertx.core.spi.metrics.ClientMetrics;
22+
import io.vertx.core.tracing.TracingPolicy;
23+
import io.vertx.ext.unit.Async;
24+
import io.vertx.ext.unit.TestContext;
25+
import io.vertx.ext.unit.junit.VertxUnitRunner;
26+
import io.vertx.sqlclient.SqlConnectOptions;
27+
import io.vertx.sqlclient.impl.pool.SqlConnectionPool;
28+
import io.vertx.sqlclient.spi.DatabaseMetadata;
29+
import io.vertx.sqlclient.spi.connection.Connection;
30+
import io.vertx.sqlclient.spi.connection.ConnectionContext;
31+
import io.vertx.sqlclient.spi.connection.ConnectionFactory;
32+
import io.vertx.sqlclient.spi.protocol.CommandBase;
33+
import org.junit.After;
34+
import org.junit.Before;
35+
import org.junit.Test;
36+
import org.junit.runner.RunWith;
37+
38+
import java.util.concurrent.atomic.AtomicBoolean;
39+
40+
/**
41+
* When the {@code afterAcquire} hook fails, the acquisition must fail with the hook failure and
42+
* the lease must be returned to the pool, otherwise the slot leaks and the pool eventually starves.
43+
*/
44+
@RunWith(VertxUnitRunner.class)
45+
public class SqlConnectionPoolAcquireHookTest {
46+
47+
static class StubConnection implements Connection {
48+
49+
@Override
50+
public TracingPolicy tracingPolicy() {
51+
return TracingPolicy.IGNORE;
52+
}
53+
54+
@Override
55+
public ClientMetrics metrics() {
56+
return null;
57+
}
58+
59+
@Override
60+
public SocketAddress server() {
61+
return SocketAddress.inetSocketAddress(5432, "localhost");
62+
}
63+
64+
@Override
65+
public String database() {
66+
return "db";
67+
}
68+
69+
@Override
70+
public String user() {
71+
return "user";
72+
}
73+
74+
@Override
75+
public void init(ConnectionContext context) {
76+
}
77+
78+
@Override
79+
public void close(ConnectionContext holder, Completable<Void> promise) {
80+
promise.succeed();
81+
}
82+
83+
@Override
84+
public boolean isSsl() {
85+
return false;
86+
}
87+
88+
@Override
89+
public boolean isValid() {
90+
return true;
91+
}
92+
93+
@Override
94+
public DatabaseMetadata databaseMetadata() {
95+
return null;
96+
}
97+
98+
@Override
99+
public <R> void schedule(CommandBase<R> cmd, Completable<R> handler) {
100+
handler.fail("not supported");
101+
}
102+
}
103+
104+
private Vertx vertx;
105+
private SqlConnectionPool pool;
106+
private final AtomicBoolean hookFails = new AtomicBoolean(true);
107+
private final Exception hookFailure = new Exception("hook failure");
108+
109+
@Before
110+
public void setUp() {
111+
vertx = Vertx.vertx();
112+
ConnectionFactory<SqlConnectOptions> factory = new ConnectionFactory<SqlConnectOptions>() {
113+
@Override
114+
public Future<Connection> connect(Context context, SqlConnectOptions options) {
115+
return Future.succeededFuture(new StubConnection());
116+
}
117+
@Override
118+
public void close(Completable<Void> completion) {
119+
completion.succeed();
120+
}
121+
};
122+
pool = new SqlConnectionPool(
123+
() -> Future.succeededFuture(new SqlConnectOptions()),
124+
factory,
125+
null,
126+
null,
127+
conn -> hookFails.get() ? Future.failedFuture(hookFailure) : Future.succeededFuture(),
128+
conn -> Future.succeededFuture(),
129+
(VertxInternal) vertx,
130+
0,
131+
0,
132+
1,
133+
false,
134+
-1,
135+
0);
136+
}
137+
138+
@After
139+
public void tearDown(TestContext ctx) {
140+
pool.close().onComplete(ctx.asyncAssertSuccess(v -> {
141+
vertx.close().onComplete(ctx.asyncAssertSuccess());
142+
}));
143+
}
144+
145+
@Test
146+
public void testAcquireFailsWithHookFailureAndReleasesTheSlot(TestContext ctx) {
147+
Async async = ctx.async();
148+
ContextInternal context = (ContextInternal) vertx.getOrCreateContext();
149+
pool.acquire(context, 0, (conn, err) -> {
150+
// the acquisition must report the hook failure, not null
151+
ctx.assertEquals(hookFailure, err);
152+
hookFails.set(false);
153+
// the pool has a single slot: this acquisition succeeds only if the failed one was released
154+
pool.acquire(context, 0, (conn2, err2) -> {
155+
ctx.assertNull(err2);
156+
ctx.assertNotNull(conn2);
157+
async.complete();
158+
});
159+
});
160+
}
161+
}

0 commit comments

Comments
 (0)