AG-154 - SQLException during reset leaks connection

agroal · Feb 17, 2021 · 1929c97 · 1929c97
1 parent 455b45d
commit 1929c97
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 9 deletions.
diff --git a/agroal-pool/src/main/java/io/agroal/pool/ConnectionHandler.java b/agroal-pool/src/main/java/io/agroal/pool/ConnectionHandler.java
@@ -132,16 +132,20 @@ public void resetConnection() throws SQLException {
             dirtyAttributes.clear();
         }
 
-        SQLWarning warning = connection.getWarnings();
-        if ( warning != null ) {
-            AgroalConnectionPoolConfiguration.ExceptionSorter exceptionSorter = connectionPool.getConfiguration().exceptionSorter();
-            while ( warning != null ) {
-                if ( exceptionSorter != null && exceptionSorter.isFatal( warning ) ) {
-                    setState( State.FLUSH );
+        try {
+            SQLWarning warning = connection.getWarnings();
+            if ( warning != null ) {
+                AgroalConnectionPoolConfiguration.ExceptionSorter exceptionSorter = connectionPool.getConfiguration().exceptionSorter();
+                while ( warning != null ) {
+                    if ( exceptionSorter != null && exceptionSorter.isFatal( warning ) ) {
+                        setState( State.FLUSH );
+                    }
+                    warning = warning.getNextWarning();
                 }
-                warning = warning.getNextWarning();
+                connection.clearWarnings();
             }
-            connection.clearWarnings();
+        } catch ( SQLException sqlException ) {
+            // keep errors
         }
     }
 

diff --git a/agroal-pool/src/main/java/io/agroal/pool/ConnectionPool.java b/agroal-pool/src/main/java/io/agroal/pool/ConnectionPool.java
@@ -367,7 +367,11 @@ public void returnConnectionHandler(ConnectionHandler handler) throws SQLExcepti
             return;
         }
 
-        handler.resetConnection();
+        try {
+            handler.resetConnection();
+        } catch ( SQLException sqlException ) {
+            fireOnWarning( listeners, sqlException );
+        }
         localCache.get().add( handler );
         fireOnConnectionReturnInterceptor( interceptors, handler );
 

diff --git a/agroal-test/src/test/java/io/agroal/test/basic/ConnectionResetTests.java b/agroal-test/src/test/java/io/agroal/test/basic/ConnectionResetTests.java
@@ -7,6 +7,7 @@
 import io.agroal.api.configuration.AgroalConnectionFactoryConfiguration;
 import io.agroal.api.configuration.supplier.AgroalDataSourceConfigurationSupplier;
 import io.agroal.test.MockConnection;
+import io.agroal.test.MockDataSource;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.DisplayName;
@@ -32,6 +33,7 @@
 import static java.util.logging.Logger.getLogger;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * @author <a href="lbarreiro@redhat.com">Luis Barreiro</a>
@@ -145,6 +147,19 @@ private void warnings(boolean fatal) throws SQLException {
 
     // --- //
 
+    @Test
+    @DisplayName( "Test exception during reset" )
+    public void resetExceptionTest() throws SQLException {
+        try ( AgroalDataSource dataSource = AgroalDataSource.from( new AgroalDataSourceConfigurationSupplier().metricsEnabled().connectionPoolConfiguration(
+                        cp -> cp.maxSize( 1 ).connectionFactoryConfiguration( cf -> cf.connectionProviderClass( SneakyDataSource.class ) ) ) ) ) {
+            try ( Connection c = dataSource.getConnection() ) {
+                assertThrows( SQLException.class, () -> c.getWarnings() );
+            }
+        }
+    }
+
+    // --- //
+
     public static class FakeConnection implements MockConnection {
 
         private int isolation;
@@ -181,4 +196,22 @@ public void clearWarnings() throws SQLException {
             this.warnings = false;
         }
     }
+
+    public static class SneakyDataSource implements MockDataSource {
+
+        @Override
+        public Connection getConnection() throws SQLException {
+            return new SneakyConnection();
+        }
+
+    }
+
+    public static class SneakyConnection implements MockConnection {
+        @Override
+        public SQLWarning getWarnings() throws SQLException {
+            // getWarnings method is called on connection return. Need to make sure the pool is usable in that scenario.
+            throw new SQLException("This one is sneaky!");
+        }
+    }
+
 }