Merge pull request #496 from nitincchauhan/dev

do not add cons if pool is not normal, avoid exceptions in shutdown

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

@@ -42,6 +42,8 @@ import com.codahale.metrics.health.HealthCheckRegistry;
 import com.zaxxer.hikari.metrics.MetricsTrackerFactory;
 import com.zaxxer.hikari.util.PropertyElf;
 
+import static com.zaxxer.hikari.util.UtilityElf.nullOrNotEmpty;
+
 public class HikariConfig implements HikariConfigMXBean
 {
    private static final Logger LOGGER = LoggerFactory.getLogger(HikariConfig.class);
@@ -746,6 +748,16 @@ public class HikariConfig implements HikariConfigMXBean
    {
       validateNumerics();
 
+      // treat empty property as null
+      catalog = nullOrNotEmpty(catalog);
+      connectionInitSql = nullOrNotEmpty(connectionInitSql);
+      connectionTestQuery = nullOrNotEmpty(connectionTestQuery);
+      transactionIsolationName = nullOrNotEmpty(transactionIsolationName);
+      dataSourceClassName = nullOrNotEmpty(dataSourceClassName);
+      dataSourceJndiName = nullOrNotEmpty(dataSourceJndiName);
+      driverClassName = nullOrNotEmpty(driverClassName);
+      jdbcUrl = nullOrNotEmpty(jdbcUrl);
+
       if (poolName == null) {
          poolName = "HikariPool-" + POOL_NUMBER.getAndIncrement();
       }

src/main/java/com/zaxxer/hikari/metrics/PoolStats.java

@@ -38,7 +38,7 @@ public abstract class PoolStats
    public PoolStats(final long timeoutMs)
    {
       this.timeoutMs = timeoutMs;
-      this.reloadAt = new AtomicLong(0);
+      this.reloadAt = new AtomicLong();
       this.clock = ClockSource.INSTANCE;
    }
    

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

@@ -105,7 +105,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL
       this.totalConnections = new AtomicInteger();
       this.suspendResumeLock = config.isAllowPoolSuspension() ? new SuspendResumeLock() : SuspendResumeLock.FAUX_LOCK;
 
-      this.addConnectionExecutor = createThreadPoolExecutor(config.getMaximumPoolSize(), "Hikari connection filler (pool " + poolName + ")", config.getThreadFactory(), new ThreadPoolExecutor.DiscardPolicy());
+      this.addConnectionExecutor = createThreadPoolExecutor(config.getMaximumPoolSize(), "Hikari connection adder (pool " + poolName + ")", config.getThreadFactory(), new ThreadPoolExecutor.DiscardPolicy());
       this.closeConnectionExecutor = createThreadPoolExecutor(4, "Hikari connection closer (pool " + poolName + ")", config.getThreadFactory(), new ThreadPoolExecutor.CallerRunsPolicy());
 
       if (config.getScheduledExecutorService() == null) {
@@ -213,8 +213,8 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL
          LOGGER.info("{} - is closing down.", poolName);
          logPoolState("Before closing\t");
 
-         connectionBag.close();
          softEvictConnections();
+
          addConnectionExecutor.shutdown();
          addConnectionExecutor.awaitTermination(5L, TimeUnit.SECONDS);
          if (config.getScheduledExecutorService() == null) {
@@ -222,13 +222,15 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL
             houseKeepingExecutorService.awaitTermination(5L, TimeUnit.SECONDS);
          }
 
-         final ExecutorService assassinExecutor = createThreadPoolExecutor(config.getMaximumPoolSize(), "Hikari connection assassin",
+         connectionBag.close();
+
+         final ExecutorService assassinExecutor = createThreadPoolExecutor(config.getMaximumPoolSize(), "Hikari connection assassin (pool " + poolName + ")",
                                                                            config.getThreadFactory(), new ThreadPoolExecutor.CallerRunsPolicy());
          try {
             final long start = clockSource.currentTime();
             do {
-               softEvictConnections();
                abortActiveConnections(assassinExecutor);
+               softEvictConnections();
             } while (getTotalConnections() > 0 && clockSource.elapsedMillis(start) < TimeUnit.SECONDS.toMillis(5));
          }
          finally {
@@ -561,7 +563,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL
       public Boolean call() throws Exception
       {
          long sleepBackoff = 200L;
-         while (totalConnections.get() < config.getMaximumPoolSize()) {
+         while (poolState == POOL_NORMAL && totalConnections.get() < config.getMaximumPoolSize()) {
             final PoolEntry poolEntry = createPoolEntry();
             if (poolEntry != null) {
                totalConnections.incrementAndGet();
@@ -573,7 +575,7 @@ public class HikariPool extends PoolBase implements HikariPoolMXBean, IBagStateL
             quietlySleep(sleepBackoff);
             sleepBackoff = Math.min(connectionTimeout / 2, (long) (sleepBackoff * 1.3));
          }
-         // Pool is at max size
+         // Pool is suspended or shutdown or at max size
          return Boolean.FALSE;
       }
    }

src/main/java/com/zaxxer/hikari/util/ConcurrentBag.java

@@ -94,7 +94,7 @@ public class ConcurrentBag<T extends IConcurrentBagEntry> implements AutoCloseab
       this.listener = listener;
       this.weakThreadLocals = useWeakThreadLocals();
 
-      this.waiters = new AtomicInteger(0);
+      this.waiters = new AtomicInteger();
       this.sharedList = new CopyOnWriteArrayList<>();
       this.synchronizer = new QueuedSequenceSynchronizer();
       if (weakThreadLocals) {

src/main/java/com/zaxxer/hikari/util/UtilityElf.java

@@ -31,6 +31,15 @@ import java.util.concurrent.TimeUnit;
  */
 public final class UtilityElf
 {
+   /**
+    *
+    * @return null if string is null or empty
+   */
+   public static String nullOrNotEmpty(final String text)
+   {
+      return text == null ? null : text.trim().isEmpty() ? null : text.trim();
+   }
+
    /**
     * Sleep and transform an InterruptedException into a RuntimeException.
     *

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

@@ -31,6 +31,7 @@ import com.zaxxer.hikari.HikariDataSource;
 import com.zaxxer.hikari.mocks.StubConnection;
 import com.zaxxer.hikari.util.ClockSource;
 import com.zaxxer.hikari.util.UtilityElf;
+import org.apache.logging.log4j.Level;
 
 /**
  * @author Brett Wooldridge
@@ -40,6 +41,7 @@ public class ShutdownTest
    @Before
    public void beforeTest()
    {
+      TestElf.setSlf4jLogLevel(ShutdownTest.class, Level.DEBUG);
       StubConnection.count.set(0);
    }
 
@@ -87,13 +89,13 @@ public class ShutdownTest
 
       UtilityElf.quietlySleep(1200L);
 
-      Assert.assertTrue("Totals connection count not as expected, ", pool.getTotalConnections() > 0);
+      Assert.assertTrue("Total connection count not as expected, ", pool.getTotalConnections() > 0);
 
       ds.close();
 
       Assert.assertSame("Active connection count not as expected, ", 0, pool.getActiveConnections());
       Assert.assertSame("Idle connection count not as expected, ", 0, pool.getIdleConnections());
-      Assert.assertSame("Total connection count not as expected", 0, pool.getTotalConnections());
+      Assert.assertSame("Total connection count not as expected, ", 0, pool.getTotalConnections());
       Assert.assertTrue(ds.isClosed());
    }
 
@@ -116,13 +118,13 @@ public class ShutdownTest
 
       UtilityElf.quietlySleep(1200L);
 
-      Assert.assertTrue("Totals connection count not as expected, ", pool.getTotalConnections() > 0);
+      Assert.assertTrue("Total connection count not as expected, ", pool.getTotalConnections() > 0);
 
       ds.close();
 
       Assert.assertSame("Active connection count not as expected, ", 0, pool.getActiveConnections());
       Assert.assertSame("Idle connection count not as expected, ", 0, pool.getIdleConnections());
-      Assert.assertSame("Total connection count not as expected", 0, pool.getTotalConnections());
+      Assert.assertSame("Total connection count not as expected, ", 0, pool.getTotalConnections());
       Assert.assertTrue(ds.toString().startsWith("HikariDataSource (") && ds.toString().endsWith(")"));
    }
 
@@ -145,13 +147,13 @@ public class ShutdownTest
 
       UtilityElf.quietlySleep(1200L);
 
-      Assert.assertTrue("Totals connection count not as expected, ", pool.getTotalConnections() == 5);
+      Assert.assertTrue("Total connection count not as expected, ", pool.getTotalConnections() == 5);
 
       ds.close();
 
       Assert.assertSame("Active connection count not as expected, ", 0, pool.getActiveConnections());
       Assert.assertSame("Idle connection count not as expected, ", 0, pool.getIdleConnections());
-      Assert.assertSame("Total connection count not as expected", 0, pool.getTotalConnections());
+      Assert.assertSame("Total connection count not as expected, ", 0, pool.getTotalConnections());
    }
 
    @Test
@@ -200,13 +202,13 @@ public class ShutdownTest
          connections[i] = ds.getConnection();
       }
 
-      Assert.assertTrue("Totals connection count not as expected, ", pool.getTotalConnections() == 5);
+      Assert.assertTrue("Total connection count not as expected, ", pool.getTotalConnections() == 5);
 
       ds.close();
 
       Assert.assertSame("Active connection count not as expected, ", 0, pool.getActiveConnections());
       Assert.assertSame("Idle connection count not as expected, ", 0, pool.getIdleConnections());
-      Assert.assertSame("Total connection count not as expected", 0, pool.getTotalConnections());
+      Assert.assertSame("Total connection count not as expected, ", 0, pool.getTotalConnections());
    }
 
    @Test