ProxyConnection always resets the underlying connection

When closing transactions with uncommited statements the proxy can
return the actual connection in an invalid state back to the pool.
Moving the connection reset into a finally block ensures that it is
always returned in a valid state.

src/main/java/com/zaxxer/hikari/pool/ProxyConnection.java

@@ -236,18 +236,21 @@ public abstract class ProxyConnection implements Connection
          leakTask.cancel();
 
          try {
-            if (isCommitStateDirty && !isAutoCommit) {
-               delegate.rollback();
-               lastAccess = currentTime();
-               LOGGER.debug("{} - Executed rollback on connection {} due to dirty commit state on close().", poolEntry.getPoolName(), delegate);
+            try {
+               if (isCommitStateDirty && !isAutoCommit) {
+                  delegate.rollback();
+                  lastAccess = currentTime();
+                  LOGGER.debug("{} - Executed rollback on connection {} due to dirty commit state on close().", poolEntry.getPoolName(), delegate);
+               }
             }
+            finally {
+               if (dirtyBits != 0) {
+                  poolEntry.resetConnectionState(this, dirtyBits);
+                  lastAccess = currentTime();
+               }
 
-            if (dirtyBits != 0) {
-               poolEntry.resetConnectionState(this, dirtyBits);
-               lastAccess = currentTime();
+               delegate.clearWarnings();
             }
-
-            delegate.clearWarnings();
          }
          catch (SQLException e) {
             // when connections are aborted, exceptions are often thrown that should not reach the application

src/test/java/com/zaxxer/hikari/pool/ConnectionStateTest.java

@@ -22,12 +22,15 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.fail;
 
 import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
 
+import com.zaxxer.hikari.mocks.StubConnection;
+import com.zaxxer.hikari.mocks.StubDataSource;
 import org.junit.Test;
 
 import com.zaxxer.hikari.HikariConfig;
@@ -167,4 +170,51 @@ public class ConnectionStateTest
          }
       }
    }
+
+   @Test
+   public void testFailedRollbackResetsProxy() throws SQLException
+   {
+      try (HikariDataSource ds = newHikariDataSource()) {
+         ds.setAutoCommit(true);
+         ds.setMinimumIdle(1);
+         ds.setMaximumPoolSize(1);
+         ds.setConnectionTestQuery("VALUES 1");
+         ds.setDataSourceClassName(RollbackResetTestDataSource.class.getName());
+
+         boolean caught = false;
+         try (Connection connection = ds.getConnection()) {
+            Connection unwrap = connection.unwrap(Connection.class);
+            Statement statement = connection.createStatement();
+            connection.setAutoCommit(false);
+
+            statement.executeUpdate("INSERT INTO something (foo) VALUES (1)");
+
+            assertFalse(unwrap.getAutoCommit());
+         } catch (SQLException e) {
+            caught = true;
+         }
+         if (!caught) {
+            fail();
+         }
+
+         try (Connection connection = ds.getConnection()) {
+            Connection unwrap = connection.unwrap(Connection.class);
+            assertTrue(unwrap.getAutoCommit());
+         }
+      }
+   }
+
+   public static class RollbackResetTestDataSource extends StubDataSource {
+      @Override
+      public Connection getConnection() throws SQLException {
+         return new RollbackResetTestConnection();
+      }
+   }
+
+   private static class RollbackResetTestConnection extends StubConnection {
+      @Override
+      public void rollback() throws SQLException {
+         throw new SQLException();
+      }
+   }
 }