The built-in connection pool (JdbcConnectionPool) did not always honor the login…

The built-in connection pool (JdbcConnectionPool) did not always honor the login timeout (the timeout could occur much too early).

h2/src/docsrc/html/changelog.html

@@ -18,10 +18,12 @@ Change Log
 <h1>Change Log</h1>
 
 <h2>Next Version (unreleased)</h2>
-<ul><li>The table INFORMATION_SCHEMA.SETTINGS now contains all H2-specific system properties 
+<ul><li>The built-in connection pool (JdbcConnectionPool) did not always honor the login timeout
+    (the timeout could occur much too early). Thanks a lot to Dario Fassi for the patch!
+</li><li>Translation: the new messages have been translated to Spanish. Thanks a lot to Dario Fassi!
+</li><li>The table INFORMATION_SCHEMA.SETTINGS now contains all H2-specific system properties 
     (the ones that start with "h2.") and that are explicitly set. Previously, some H2-specific settings
     (for example h2.analyzeAuto) were missing in this list.
-</li><li>Translation: the new messages have been translated to Spanish. Thanks a lot to Dario Fassi!
 </li><li>EXPLAIN ANALYZE with an in-memory database threw an exception. Issue 216.
 </li><li>Data modifications (inserts, updates, and deletes) are now up to 5 times faster
     because converting objects to byte arrays is avoided if possible.

h2/src/main/org/h2/jdbcx/JdbcConnectionPool.java

@@ -65,11 +65,12 @@ import org.h2.message.DbException;
 public class JdbcConnectionPool implements DataSource, ConnectionEventListener {
 
     private static final int DEFAULT_TIMEOUT = 30;
+    private static final int DEFAULT_MAX_CONNECTIONS = 10;
 
     private final ConnectionPoolDataSource dataSource;
     private final Stack<PooledConnection> recycledConnections = new Stack<PooledConnection>();
     private PrintWriter logWriter;
-    private int maxConnections = 10;
+    private int maxConnections = DEFAULT_MAX_CONNECTIONS;
     private int timeout = DEFAULT_TIMEOUT;
     private int activeConnections;
     private boolean isDisposed;
@@ -144,7 +145,7 @@ public class JdbcConnectionPool implements DataSource, ConnectionEventListener {
 
     /**
      * Sets the maximum time in seconds to wait for a free connection.
-     * The default timeout is 5 minutes. Calling this method with the
+     * The default timeout is 30 seconds. Calling this method with the
      * value 0 will set the timeout to the default value.
      *
      * @param seconds the timeout, 0 meaning the default
@@ -194,14 +195,12 @@ public class JdbcConnectionPool implements DataSource, ConnectionEventListener {
      *      or a timeout occurred
      */
     public Connection getConnection() throws SQLException {
-        for (int i = 0;; i++) {
+        long max = System.currentTimeMillis() + timeout * 1000;
+        while (System.currentTimeMillis() <= max) {
             synchronized (this) {
                 if (activeConnections < maxConnections) {
                     return getConnectionNow();
                 }
-                if (i >= timeout) {
-                    throw new SQLException("Login timeout", "08001", 8001);
-                }
                 try {
                     wait(1000);
                 } catch (InterruptedException e) {
@@ -209,6 +208,7 @@ public class JdbcConnectionPool implements DataSource, ConnectionEventListener {
                 }
             }
         }
+        throw new SQLException("Login timeout", "08001", 8001);
     }
 
     private Connection getConnectionNow() throws SQLException {

h2/src/test/org/h2/test/jdbcx/TestConnectionPool.java

@@ -33,6 +33,7 @@ public class TestConnectionPool extends TestBase {
 
     public void test() throws Exception {
         deleteDb("connectionPool");
+        testTimeout();
         testUncommittedTransaction();
         testPerformance();
         testKeepOpen();
@@ -41,6 +42,40 @@ public class TestConnectionPool extends TestBase {
         deleteDb("connectionPool");
     }
 
+    private void testTimeout() throws Exception {
+        String url = getURL("connectionPool", true), user = getUser(), password = getPassword();
+        final JdbcConnectionPool man = JdbcConnectionPool.create(url, user, password);
+        man.setLoginTimeout(1);
+        man.setMaxConnections(2);
+        Connection conn = man.getConnection();
+        final boolean[] stop = { false };
+        Thread t = new Thread() {
+            public void run() {
+                while (!stop[0]) {
+                    // this calls notifyAll
+                    man.setMaxConnections(1);
+                    man.setMaxConnections(2);
+                }
+            }
+        };
+        t.start();
+        long time = System.currentTimeMillis();
+        try {
+            man.getConnection();
+            man.getConnection();
+            fail();
+        } catch (SQLException e) {
+            time = System.currentTimeMillis() - time;
+            assertTrue("timeout after " + time + " ms", time > 1000);
+        } finally {
+            conn.close();
+            stop[0] = true;
+            t.join();
+        }
+
+        man.dispose();
+    }
+
     private void testUncommittedTransaction() throws SQLException {
         String url = getURL("connectionPool", true), user = getUser(), password = getPassword();
         JdbcConnectionPool man = JdbcConnectionPool.create(url, user, password);