CachedConnection.getConnection treats every SQLException from DriverManager.getConnection as pool exhaustion and recurses with a doubling wait. The recursion has no bound, so a wrong password, an unreachable host or a database that is down never surfaces as an error — the caller simply never returns.
Surfaced while reviewing #867; it does not belong to that PR.
The loop
// opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java:82
static Connection getConnection(String connectionString, final int waitTime) throws Exception {
CachedConnection con = cached.get(connectionString).poll(waitTime, TimeUnit.MILLISECONDS);
while (con != null) { ... }
try {
final Connection conNew = DriverManager.getConnection(connectionString);
conNew.setAutoCommit(false);
conNew.setTransactionIsolation(TRANSACTION_READ_COMMITTED);
return new CachedConnection(connectionString, conNew);
} catch (SQLException e) { // max_connection server error: try recursion for reuse connection
return getConnection(connectionString, (waitTime == 0) ? 1 : waitTime * 2);
}
}Two problems, both in that one catch.
The condition is not checked. The comment names max_connections, but the catch takes any SQLException: bad credentials (28000), unknown database, a refused TCP connection, an expired certificate. None of them clears by waiting, and none of them is ever reported. e is not even logged, so there is nothing in the log to explain the hang.
There is no bound. waitTime doubles per attempt, so the wait is roughly 2^n ms: about a second by attempt 10, seventeen minutes by attempt 20, eleven days by attempt 30. In practice the call simply never returns. Being recursion rather than a loop, it also grows the stack by one frame per attempt, but the delays reach absurd values long before the stack does, so the visible symptom is a hang, not a StackOverflowError.
The wait itself is spent in poll(waitTime, ...), so a connection released by another thread is picked up — which is the case the code was written for and does work.
Effect
A JDBC backend whose credentials are wrong, or whose database is down at startup, blocks the thread that opened it instead of failing initialization with the driver's message. JDBCStorage.open() and every read/write go through this method, so the same applies at runtime after the database goes away.
Suggested fix
Bound the retry and narrow it to the condition it was written for: keep retrying while the pool is plausibly exhausted, up to a wall-clock deadline, and rethrow anything else at once. Logging e at debug on each attempt and at warn when the deadline expires would make the difference between "waiting for a connection" and "cannot connect" visible in the log, which today it is not.
CachedConnection.getConnectiontreats everySQLExceptionfromDriverManager.getConnectionas pool exhaustion and recurses with a doubling wait. The recursion has no bound, so a wrong password, an unreachable host or a database that is down never surfaces as an error — the caller simply never returns.Surfaced while reviewing #867; it does not belong to that PR.
The loop
Two problems, both in that one
catch.The condition is not checked. The comment names
max_connections, but thecatchtakes anySQLException: bad credentials (28000), unknown database, a refused TCP connection, an expired certificate. None of them clears by waiting, and none of them is ever reported.eis not even logged, so there is nothing in the log to explain the hang.There is no bound.
waitTimedoubles per attempt, so the wait is roughly2^nms: about a second by attempt 10, seventeen minutes by attempt 20, eleven days by attempt 30. In practice the call simply never returns. Being recursion rather than a loop, it also grows the stack by one frame per attempt, but the delays reach absurd values long before the stack does, so the visible symptom is a hang, not aStackOverflowError.The wait itself is spent in
poll(waitTime, ...), so a connection released by another thread is picked up — which is the case the code was written for and does work.Effect
A JDBC backend whose credentials are wrong, or whose database is down at startup, blocks the thread that opened it instead of failing initialization with the driver's message.
JDBCStorage.open()and everyread/writego through this method, so the same applies at runtime after the database goes away.Suggested fix
Bound the retry and narrow it to the condition it was written for: keep retrying while the pool is plausibly exhausted, up to a wall-clock deadline, and rethrow anything else at once. Logging
eat debug on each attempt and at warn when the deadline expires would make the difference between "waiting for a connection" and "cannot connect" visible in the log, which today it is not.