Skip to content

Commit

Permalink
Manage in-ctor exceptions
Browse files Browse the repository at this point in the history
If the c'tor of a handle throws an exception, ensure that the underlying
connection is closed as the caller will not have a chance to do so with
try-with-resources

This fixes the cause of jdbi#2446
  • Loading branch information
hgschmie committed Aug 8, 2023
1 parent 473b57a commit 61b3fcb
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 4 deletions.
14 changes: 11 additions & 3 deletions core/src/main/java/org/jdbi/v3/core/Handle.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,17 @@ private Handle(Jdbi jdbi,
this.statementBuilder = statementBuilder;
this.handleListeners = getConfig().get(Handles.class).copyListeners();

// both of these methods are bad because they leak a reference to this handle before the c'tor finished.
this.transactionHandler = transactionHandler.specialize(this);
this.forceEndTransactions = !transactionHandler.isInTransaction(this);

try {
// both of these methods are bad because they leak a reference to this handle before the c'tor finished.
this.transactionHandler = transactionHandler.specialize(this);
this.forceEndTransactions = !transactionHandler.isInTransaction(this);
} catch (SQLException e) {
// at this point, the handle is no longer viable. Close the connection
// and rethrow the exception
connectionCleaner.close();
throw e;
}

addCleanable(() ->
statementBuilder.close(connection));
Expand Down
32 changes: 32 additions & 0 deletions core/src/test/java/org/jdbi/v3/core/TestHandle.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@
*/
package org.jdbi.v3.core;

import java.sql.SQLException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;

import org.jdbi.v3.core.junit5.H2DatabaseExtension;
import org.jdbi.v3.core.statement.UnableToCreateStatementException;
import org.jdbi.v3.core.transaction.LocalTransactionHandler;
import org.jdbi.v3.core.transaction.TransactionException;
import org.jdbi.v3.core.transaction.TransactionHandler;
import org.jdbi.v3.core.transaction.TransactionIsolationLevel;
import org.jdbi.v3.core.transaction.UnableToManipulateTransactionIsolationLevelException;
import org.junit.jupiter.api.AfterEach;
Expand Down Expand Up @@ -194,6 +196,29 @@ public void testIssue2065() throws Exception {
assertThat(value).isEqualTo("Brian");
}

@Test
public void testIssue2446() throws Exception {

h2Extension.getSharedHandle().execute("insert into something (id, name) values (1, 'Brian')");

assertThatThrownBy(() -> {
String value;
Jdbi jdbi = h2Extension.getJdbi();
final ExplodeInSpecializeTransactionHandler handler = new ExplodeInSpecializeTransactionHandler();
h2Extension.getJdbi().setTransactionHandler(handler);
try (Handle handle = jdbi.open()) {
value = handle.createQuery("select name from something where id = 1").mapToBean(Something.class).one().getName();
}
assertThat(value).isEqualTo("Brian");
}).isInstanceOf(ConnectionException.class)
.hasMessageContaining("transaction specialization failure!")
.hasCauseInstanceOf(SQLException.class);

// see if the c'tor leaked a connection
assertThat(h2Extension.getLastConnection()).isNotNull();
assertThat(h2Extension.getLastConnection().isClosed()).isTrue();
}

@Test
public void testUseHandleUsesSameHandle() {
Jdbi jdbi = h2Extension.getJdbi();
Expand Down Expand Up @@ -287,4 +312,11 @@ private void maybeFail(boolean fail) {
}
}
}

static class ExplodeInSpecializeTransactionHandler extends LocalTransactionHandler {
@Override
public TransactionHandler specialize(Handle handle) throws SQLException {
throw new SQLException("transaction specialization failure!");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
package org.jdbi.v3.core.junit5;

import java.sql.Connection;
import java.sql.DriverManager;
import java.util.LinkedHashSet;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -49,6 +51,8 @@ public final class H2DatabaseExtension implements DatabaseExtension<H2DatabaseEx
private final boolean installPlugins;
private Optional<DatabaseInitializer> initializerMaybe = Optional.empty();

private volatile Connection lastConnection = null;

private Jdbi jdbi = null;
private Handle sharedHandle = null;
private boolean enableLeakchecker = true;
Expand Down Expand Up @@ -91,6 +95,10 @@ public Handle getSharedHandle() {
return sharedHandle;
}

public Connection getLastConnection() {
return lastConnection;
}

@Override
public H2DatabaseExtension withPlugin(JdbiPlugin plugin) {
plugins.add(plugin);
Expand Down Expand Up @@ -119,7 +127,10 @@ public void beforeEach(ExtensionContext context) throws Exception {
if (jdbi != null) {
throw new IllegalStateException("jdbi is not null!");
}
jdbi = Jdbi.create(uri);
jdbi = Jdbi.create(() -> {
this.lastConnection = DriverManager.getConnection(uri);
return lastConnection;
});

installTestPlugins(jdbi);

Expand Down

0 comments on commit 61b3fcb

Please sign in to comment.