Possibly Fixes #606 Fixes ##610 housekeeper was running before all class members were initialised leading to an NPE in the housekeeper. Whether or not a particular VM would recover depends somewhat on its memory model. The pool on OS X did recover after the initial failure.

pom.xml

@@ -187,6 +187,12 @@
          <version>${pax.url.version}</version>
          <scope>test</scope>
       </dependency>
+      <dependency>
+         <groupId>com.h2database</groupId>
+         <artifactId>h2</artifactId>
+         <version>1.4.191</version>
+         <scope>test</scope>
+      </dependency>
    </dependencies>
 
    <build>

src/main/java/com/zaxxer/hikari/HikariConfig.java

@@ -53,6 +53,7 @@ public class HikariConfig implements HikariConfigMXBean
    private static final long VALIDATION_TIMEOUT = SECONDS.toMillis(5);
    private static final long IDLE_TIMEOUT = MINUTES.toMillis(10);
    private static final long MAX_LIFETIME = MINUTES.toMillis(30);
+   private static final int DEFAULT_POOL_SIZE = 10;
 
    private static boolean unitTest;
 
@@ -825,7 +826,7 @@ public class HikariConfig implements HikariConfigMXBean
       }
 
       if (maxPoolSize < 1) {
-         maxPoolSize = (minIdle <= 0) ? 10 : minIdle;
+         maxPoolSize = (minIdle <= 0) ? DEFAULT_POOL_SIZE : minIdle;
       }
 
       if (minIdle < 0 || minIdle > maxPoolSize) {

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

@@ -131,9 +131,9 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL
          this.houseKeepingExecutorService = config.getScheduledExecutorService();
       }
 
-      this.houseKeepingExecutorService.scheduleWithFixedDelay(new HouseKeeper(), 0L, HOUSEKEEPING_PERIOD_MS, MILLISECONDS);
-
       this.leakTask = new ProxyLeakTask(config.getLeakDetectionThreshold(), houseKeepingExecutorService);
+
+      this.houseKeepingExecutorService.scheduleWithFixedDelay(new HouseKeeper(), 100L, HOUSEKEEPING_PERIOD_MS, MILLISECONDS);
    }
 
    /**
@@ -587,54 +587,59 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL
       @Override
       public void run()
       {
-         // refresh timeouts in case they changed via MBean
-         connectionTimeout = config.getConnectionTimeout();
-         validationTimeout = config.getValidationTimeout();
-         leakTask.updateLeakDetectionThreshold(config.getLeakDetectionThreshold());
-
-         final long idleTimeout = config.getIdleTimeout();
-         final long now = clockSource.currentTime();
-
-         // Detect retrograde time, allowing +128ms as per NTP spec.
-         if (clockSource.plusMillis(now, 128) < clockSource.plusMillis(previous, HOUSEKEEPING_PERIOD_MS)) {
-            LOGGER.warn("{} - Retrograde clock change detected (housekeeper delta={}), soft-evicting connections from pool.",
-                        clockSource.elapsedDisplayString(previous, now), poolName);
+         try {
+            // refresh timeouts in case they changed via MBean
+            connectionTimeout = config.getConnectionTimeout();
+            validationTimeout = config.getValidationTimeout();
+            leakTask.updateLeakDetectionThreshold(config.getLeakDetectionThreshold());
+   
+            final long idleTimeout = config.getIdleTimeout();
+            final long now = clockSource.currentTime();
+   
+            // Detect retrograde time, allowing +128ms as per NTP spec.
+            if (clockSource.plusMillis(now, 128) < clockSource.plusMillis(previous, HOUSEKEEPING_PERIOD_MS)) {
+               LOGGER.warn("{} - Retrograde clock change detected (housekeeper delta={}), soft-evicting connections from pool.",
+                           clockSource.elapsedDisplayString(previous, now), poolName);
+               previous = now;
+               softEvictConnections();
+               fillPool();
+               return;
+            }
+            else if (now > clockSource.plusMillis(previous, (3 * HOUSEKEEPING_PERIOD_MS) / 2)) {
+               // No point evicting for forward clock motion, this merely accelerates connection retirement anyway
+               LOGGER.warn("{} - Thread starvation or clock leap detected (housekeeper delta={}).", clockSource.elapsedDisplayString(previous, now), poolName);
+            }
+   
             previous = now;
-            softEvictConnections();
-            fillPool();
-            return;
-         }
-         else if (now > clockSource.plusMillis(previous, (3 * HOUSEKEEPING_PERIOD_MS) / 2)) {
-            // No point evicting for forward clock motion, this merely accelerates connection retirement anyway
-            LOGGER.warn("{} - Thread starvation or clock leap detected (housekeeper delta={}).", clockSource.elapsedDisplayString(previous, now), poolName);
-         }
    
-         previous = now;
-
-         String afterPrefix = "Pool ";
-         if (idleTimeout > 0L) {
-            final List<PoolEntry> idleList = connectionBag.values(STATE_NOT_IN_USE);
-            int removable = idleList.size() - config.getMinimumIdle();
-            if (removable > 0) {
-               logPoolState("Before cleanup ");
-               afterPrefix = "After cleanup  ";
-
-               // Sort pool entries on lastAccessed
-               Collections.sort(idleList, LASTACCESS_COMPARABLE);
-               for (PoolEntry poolEntry : idleList) {
-                  if (clockSource.elapsedMillis(poolEntry.lastAccessed, now) > idleTimeout && connectionBag.reserve(poolEntry)) {
-                     closeConnection(poolEntry, "(connection has passed idleTimeout)");
-                     if (--removable == 0) {
-                        break; // keep min idle cons
+            String afterPrefix = "Pool ";
+            if (idleTimeout > 0L) {
+               final List<PoolEntry> idleList = connectionBag.values(STATE_NOT_IN_USE);
+               int removable = idleList.size() - config.getMinimumIdle();
+               if (removable > 0) {
+                  logPoolState("Before cleanup ");
+                  afterPrefix = "After cleanup  ";
+   
+                  // Sort pool entries on lastAccessed
+                  Collections.sort(idleList, LASTACCESS_COMPARABLE);
+                  for (PoolEntry poolEntry : idleList) {
+                     if (clockSource.elapsedMillis(poolEntry.lastAccessed, now) > idleTimeout && connectionBag.reserve(poolEntry)) {
+                        closeConnection(poolEntry, "(connection has passed idleTimeout)");
+                        if (--removable == 0) {
+                           break; // keep min idle cons
+                        }
                      }
                   }
                }
             }
-         }
    
-         logPoolState(afterPrefix);
+            logPoolState(afterPrefix);
    
-         fillPool(); // Try to maintain minimum connections
+            fillPool(); // Try to maintain minimum connections
+         }
+         catch (Exception e) {
+            LOGGER.error("Unexpected exception in housekeeping task", e);
+         }
       }
    }
 

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

@@ -12,6 +12,7 @@ import static java.util.concurrent.TimeUnit.SECONDS;
 import java.lang.management.ManagementFactory;
 import java.sql.Connection;
 import java.sql.SQLException;
+import java.sql.SQLTransientConnectionException;
 import java.sql.Statement;
 import java.util.Properties;
 import java.util.concurrent.Executor;
@@ -313,6 +314,10 @@ abstract class PoolBase
          String password = config.getPassword();
 
          connection = (username == null) ? dataSource.getConnection() : dataSource.getConnection(username, password);
+         if (connection == null) {
+            throw new SQLTransientConnectionException("DataSource returned null unexpectedly");
+         }
+
          setupConnection(connection);
          lastConnectionFailure.set(null);
          return connection;

src/test/java/com/zaxxer/hikari/db/BasicPoolTest.java

@@ -0,0 +1,144 @@
+/*
+ * Copyright (C) 2016 Brett Wooldridge
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.zaxxer.hikari.db;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.zaxxer.hikari.HikariConfig;
+import com.zaxxer.hikari.HikariDataSource;
+import com.zaxxer.hikari.pool.HikariPool;
+import com.zaxxer.hikari.pool.TestElf;
+
+/**
+ * @author brettw
+ *
+ */
+public class BasicPoolTest
+{
+   @Before
+   public void setup() throws SQLException
+   {
+       HikariConfig config = new HikariConfig();
+       config.setMinimumIdle(1);
+       config.setMaximumPoolSize(2);
+       config.setConnectionTestQuery("SELECT 1");
+       config.setDataSourceClassName("org.h2.jdbcx.JdbcDataSource");
+       config.addDataSourceProperty("url", "jdbc:h2:mem:test;DB_CLOSE_DELAY=-1");
+
+       try (HikariDataSource ds = new HikariDataSource(config);
+            Connection conn = ds.getConnection();
+            Statement stmt = conn.createStatement()) {
+          stmt.executeUpdate("DROP TABLE IF EXISTS basic_pool_test");
+          stmt.executeUpdate("CREATE TABLE basic_pool_test ("
+                            + "id INTEGER NOT NULL IDENTITY PRIMARY KEY, "
+                            + "timestamp TIMESTAMP, "
+                            + "string VARCHAR(128), "
+                            + "string_from_number NUMERIC "
+                            + ")");
+       }
+   }
+
+   @Test
+   public void testIdleTimeout() throws InterruptedException, SQLException
+   {
+      HikariConfig config = new HikariConfig();
+      config.setMinimumIdle(5);
+      config.setMaximumPoolSize(10);
+      config.setConnectionTestQuery("SELECT 1");
+      config.setDataSourceClassName("org.h2.jdbcx.JdbcDataSource");
+      config.addDataSourceProperty("url", "jdbc:h2:mem:test;DB_CLOSE_DELAY=-1");
+
+      System.setProperty("com.zaxxer.hikari.housekeeping.periodMs", "1000");
+
+      try (HikariDataSource ds = new HikariDataSource(config)) {
+         System.clearProperty("com.zaxxer.hikari.housekeeping.periodMs");
+
+         TimeUnit.SECONDS.sleep(1);
+
+         HikariPool pool = TestElf.getPool(ds);
+
+         ds.setIdleTimeout(3000);
+
+         Assert.assertSame("Total connections not as expected", 5, pool.getTotalConnections());
+         Assert.assertSame("Idle connections not as expected", 5, pool.getIdleConnections());
+
+         Connection connection = ds.getConnection();
+         Assert.assertNotNull(connection);
+
+         TimeUnit.MILLISECONDS.sleep(1500);
+
+         Assert.assertSame("Second total connections not as expected", 6, pool.getTotalConnections());
+         Assert.assertSame("Second idle connections not as expected", 5, pool.getIdleConnections());
+         connection.close();
+
+         Assert.assertSame("Idle connections not as expected", 6, pool.getIdleConnections());
+
+         TimeUnit.SECONDS.sleep(2);
+
+         Assert.assertSame("Third total connections not as expected", 5, pool.getTotalConnections());
+         Assert.assertSame("Third idle connections not as expected", 5, pool.getIdleConnections());
+      }
+   }
+
+   @Test
+   public void testIdleTimeout2() throws InterruptedException, SQLException
+   {
+      HikariConfig config = new HikariConfig();
+      config.setMaximumPoolSize(50);
+      config.setConnectionTestQuery("SELECT 1");
+      config.setDataSourceClassName("org.h2.jdbcx.JdbcDataSource");
+      config.addDataSourceProperty("url", "jdbc:h2:mem:test;DB_CLOSE_DELAY=-1");
+
+      System.setProperty("com.zaxxer.hikari.housekeeping.periodMs", "1000");
+
+      try (HikariDataSource ds = new HikariDataSource(config)) {
+         System.clearProperty("com.zaxxer.hikari.housekeeping.periodMs");
+
+         TimeUnit.SECONDS.sleep(1);
+
+         HikariPool pool = TestElf.getPool(ds);
+
+         ds.setIdleTimeout(3000);
+
+         Assert.assertSame("Total connections not as expected", 50, pool.getTotalConnections());
+         Assert.assertSame("Idle connections not as expected", 50, pool.getIdleConnections());
+
+         Connection connection = ds.getConnection();
+         Assert.assertNotNull(connection);
+
+         TimeUnit.MILLISECONDS.sleep(1500);
+
+         Assert.assertSame("Second total connections not as expected", 50, pool.getTotalConnections());
+         Assert.assertSame("Second idle connections not as expected", 49, pool.getIdleConnections());
+         connection.close();
+
+         Assert.assertSame("Idle connections not as expected", 50, pool.getIdleConnections());
+
+         TimeUnit.SECONDS.sleep(3);
+
+         Assert.assertSame("Third total connections not as expected", 50, pool.getTotalConnections());
+         Assert.assertSame("Third idle connections not as expected", 50, pool.getIdleConnections());
+      }
+   }
+}

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

@@ -508,4 +508,48 @@ public class TestConnections
          Assert.assertSame("Bad query or something.", e.getNextException().getMessage());
       }
    }
+
+   @Test
+   public void testPopulationSlowAcquisition() throws InterruptedException, SQLException
+   {
+      HikariConfig config = new HikariConfig();
+      config.setPoolName("SlowAcquisition");
+      config.setMaximumPoolSize(30);
+      config.setConnectionTestQuery("VALUES 1");
+      config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
+
+      System.setProperty("com.zaxxer.hikari.housekeeping.periodMs", "1000");
+
+      StubConnection.slowCreate = true;
+      try (HikariDataSource ds = new HikariDataSource(config)) {
+         System.clearProperty("com.zaxxer.hikari.housekeeping.periodMs");
+
+         ds.setIdleTimeout(3000);
+
+         TimeUnit.SECONDS.sleep(2);
+
+         HikariPool pool = TestElf.getPool(ds);
+         Assert.assertSame("Total connections not as expected", 1, pool.getTotalConnections());
+         Assert.assertSame("Idle connections not as expected", 1, pool.getIdleConnections());
+
+         Connection connection = ds.getConnection();
+         Assert.assertNotNull(connection);
+
+         TimeUnit.SECONDS.sleep(30);
+
+         Assert.assertSame("Second total connections not as expected", 30, pool.getTotalConnections());
+         Assert.assertSame("Second idle connections not as expected", 29, pool.getIdleConnections());
+         connection.close();
+
+         Assert.assertSame("Idle connections not as expected", 30, pool.getIdleConnections());
+
+         TimeUnit.SECONDS.sleep(5);
+
+         Assert.assertSame("Third total connections not as expected", 30, pool.getTotalConnections());
+         Assert.assertSame("Third idle connections not as expected", 30, pool.getIdleConnections());
+      }
+      finally {
+         StubConnection.slowCreate = false;
+      }
+   }
 }