Fix #198 handle pull shutdown synchronisation issues

hikaricp-java6/src/main/java/com/zaxxer/hikari/pool/HikariPool.java

@@ -313,7 +313,7 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
             long sleepBackoff = 200L;
             final int maxPoolSize = configuration.getMaximumPoolSize();
             final int minIdle = configuration.getMinimumIdle();
-            while (!isShutdown && totalConnections.get() < maxPoolSize && (minIdle == 0 || getIdleConnections() < minIdle)) {
+            while (!isPoolSuspended && !isShutdown && totalConnections.get() < maxPoolSize && (minIdle == 0 || getIdleConnections() < minIdle)) {
                if (addConnection()) {
                   if (minIdle == 0) {
                      break; // This break is here so we only add one connection when there is no min. idle
@@ -398,6 +398,7 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
       if (isPoolSuspended) {
          acquisitionSemaphore.release(10000);
          isPoolSuspended = false;
+         addBagItem(); // re-populate the pool
       }
    }
 
@@ -412,16 +413,17 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
     */
    private void closeConnection(final PoolBagEntry bagEntry)
    {
-      connectionBag.remove(bagEntry);
+      if (connectionBag.remove(bagEntry)) {
-      final int tc = totalConnections.decrementAndGet();
+         final int tc = totalConnections.decrementAndGet();
-      if (tc < 0) {
+         if (tc < 0) {
-         LOGGER.warn("Internal accounting inconsistency, totalConnections={}", tc, new Exception());
+            LOGGER.warn("Internal accounting inconsistency, totalConnections={}", tc, new Exception());
-      }
-      closeConnectionExecutor.submit(new Runnable() {
-         public void run() {
-            quietlyCloseConnection(bagEntry.connection);
          }
-      });
+         closeConnectionExecutor.submit(new Runnable() {
+            public void run() {
+               quietlyCloseConnection(bagEntry.connection);
+            }
+         });
+      }
    }
 
    /**
@@ -432,7 +434,7 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
       // Speculative increment of totalConnections with expectation of success
       if (totalConnections.incrementAndGet() > configuration.getMaximumPoolSize() || isShutdown || isPoolSuspended) {
          totalConnections.decrementAndGet();
-         return !isPoolSuspended;
+         return true;
       }
 
       Connection connection = null;
@@ -535,6 +537,7 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
       ExecutorService assassinExecutor = createThreadPoolExecutor(configuration.getMaximumPoolSize(), "HikariCP connection assassin", configuration.getThreadFactory(), new ThreadPoolExecutor.CallerRunsPolicy());
       for (PoolBagEntry bagEntry : connectionBag.values(STATE_IN_USE)) {
          try {
+            bagEntry.evicted = true;
             bagEntry.connection.abort(assassinExecutor);
          }
          catch (AbstractMethodError e) {
@@ -547,13 +550,9 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
             quietlyCloseConnection(bagEntry.connection);
          }
          finally {
-            try {
+            if (connectionBag.remove(bagEntry)) {
-               connectionBag.remove(bagEntry);
                totalConnections.decrementAndGet();
             }
-            catch (IllegalStateException ise) {
-               continue;
-            }
          }
       }
 

hikaricp-java6/src/main/java/com/zaxxer/hikari/pool/PoolBagEntry.java

@@ -30,7 +30,7 @@ public final class PoolBagEntry extends BagEntry
    public final Connection connection;
    public final long expirationTime;
    public long lastOpenTime;
-   volatile boolean evicted;
+   public volatile boolean evicted;
    long lastAccess;
 
    public PoolBagEntry(final Connection connection, long maxLifetime) {

hikaricp-java6/src/main/java/com/zaxxer/hikari/proxy/ConnectionProxy.java

@@ -202,7 +202,10 @@ public abstract class ConnectionProxy implements IHikariConnectionProxy
             delegate.clearWarnings();
          }
          catch (SQLException e) {
-            throw checkException(e);
+            // when connections are evicted, exceptions are often thrown that should not reach the application
+            if (!bagEntry.evicted) {
+               throw checkException(e);
+            }
          }
          finally {
             delegate = ClosedConnection.CLOSED_CONNECTION;

hikaricp-java6/src/main/java/com/zaxxer/hikari/util/ConcurrentBag.java

@@ -194,17 +194,20 @@ public final class ConcurrentBag<T extends BagEntry>
     * with objects obtained by {@link #borrow(long, TimeUnit)} or {@link #reserve(BagEntry)}.
     *
     * @param bagEntry the value to remove
+    * @return true if the entry was removed, false otherwise
     * @throws IllegalStateException if an attempt is made to remove an object
     *         from the bag that was not borrowed or reserved first
     */
-   public void remove(final T bagEntry)
+   public boolean remove(final T bagEntry)
    {
       if (!bagEntry.state.compareAndSet(STATE_IN_USE, STATE_REMOVED) && !bagEntry.state.compareAndSet(STATE_RESERVED, STATE_REMOVED) && !closed) {
          throw new IllegalStateException("Attempt to remove an object from the bag that was not borrowed or reserved");
       }
-      else if (!sharedList.remove(bagEntry) && !closed) {
+      final boolean removed = sharedList.remove(bagEntry);
+      if (!removed && !closed) {
          throw new IllegalStateException("Attempt to remove an object from the bag that does not exist");
       }
+      return removed;
    }
 
    /**

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

@@ -307,7 +307,7 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
          long sleepBackoff = 200L;
          final int maxPoolSize = configuration.getMaximumPoolSize();
          final int minIdle = configuration.getMinimumIdle();
-         while (!isShutdown && totalConnections.get() < maxPoolSize && (minIdle == 0 || getIdleConnections() < minIdle)) {
+         while (!isPoolSuspended && !isShutdown && totalConnections.get() < maxPoolSize && (minIdle == 0 || getIdleConnections() < minIdle)) {
             if (addConnection()) {
                if (minIdle == 0) {
                   break; // This break is here so we only add one connection when there is no min. idle
@@ -382,6 +382,7 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
       if (isPoolSuspended) {
          acquisitionSemaphore.release(10000);
          isPoolSuspended = false;
+         addBagItem(); // re-populate the pool
       }
    }
 
@@ -396,12 +397,13 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
     */
    private void closeConnection(final PoolBagEntry bagEntry)
    {
-      connectionBag.remove(bagEntry);
+      if (connectionBag.remove(bagEntry)) {
-      final int tc = totalConnections.decrementAndGet();
+         final int tc = totalConnections.decrementAndGet();
-      if (tc < 0) {
+         if (tc < 0) {
-         LOGGER.warn("Internal accounting inconsistency, totalConnections={}", tc, new Exception());
+            LOGGER.warn("Internal accounting inconsistency, totalConnections={}", tc, new Exception());
+         }
+         closeConnectionExecutor.submit(() -> { quietlyCloseConnection(bagEntry.connection); });
       }
-      closeConnectionExecutor.submit(() -> { quietlyCloseConnection(bagEntry.connection); });
    }
 
    /**
@@ -412,7 +414,7 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
       // Speculative increment of totalConnections with expectation of success
       if (totalConnections.incrementAndGet() > configuration.getMaximumPoolSize() || isShutdown || isPoolSuspended) {
          totalConnections.decrementAndGet();
-         return !isPoolSuspended;
+         return true;
       }
 
       Connection connection = null;
@@ -511,19 +513,16 @@ public final class HikariPool implements HikariPoolMBean, IBagStateListener
       ExecutorService assassinExecutor = createThreadPoolExecutor(configuration.getMaximumPoolSize(), "HikariCP connection assassin", configuration.getThreadFactory(), new ThreadPoolExecutor.CallerRunsPolicy());
       connectionBag.values(STATE_IN_USE).stream().forEach(bagEntry -> {
          try {
+            bagEntry.evicted = true;
             bagEntry.connection.abort(assassinExecutor);
          }
          catch (SQLException | AbstractMethodError e) {
             quietlyCloseConnection(bagEntry.connection);
          }
          finally {
-            try {
+            if (connectionBag.remove(bagEntry)) {
-               connectionBag.remove(bagEntry);
                totalConnections.decrementAndGet();
             }
-            catch (IllegalStateException ise) {
-               return;
-            }
          }
       });
 

hikaricp/src/main/java/com/zaxxer/hikari/pool/PoolBagEntry.java

@@ -30,7 +30,7 @@ public final class PoolBagEntry extends BagEntry
    public final Connection connection;
    public final long expirationTime;
    public long lastOpenTime;
-   volatile boolean evicted;
+   public volatile boolean evicted;
    long lastAccess;
 
    public PoolBagEntry(final Connection connection, long maxLifetime) {

hikaricp/src/main/java/com/zaxxer/hikari/proxy/ConnectionProxy.java

@@ -202,7 +202,10 @@ public abstract class ConnectionProxy implements IHikariConnectionProxy
             delegate.clearWarnings();
          }
          catch (SQLException e) {
-            throw checkException(e);
+            // when connections are evicted, exceptions are often thrown that should not reach the application
+            if (!bagEntry.evicted) {
+               throw checkException(e);
+            }
          }
          finally {
             delegate = ClosedConnection.CLOSED_CONNECTION;

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

@@ -193,17 +193,20 @@ public final class ConcurrentBag<T extends BagEntry>
     * with objects obtained by {@link #borrow(long, TimeUnit)} or {@link #reserve(BagEntry)}.
     *
     * @param bagEntry the value to remove
+    * @return true if the entry was removed, false otherwise
     * @throws IllegalStateException if an attempt is made to remove an object
     *         from the bag that was not borrowed or reserved first
     */
-   public void remove(final T bagEntry)
+   public boolean remove(final T bagEntry)
    {
       if (!bagEntry.state.compareAndSet(STATE_IN_USE, STATE_REMOVED) && !bagEntry.state.compareAndSet(STATE_RESERVED, STATE_REMOVED) && !closed) {
          throw new IllegalStateException("Attempt to remove an object from the bag that was not borrowed or reserved");
       }
-      else if (!sharedList.remove(bagEntry) && !closed) {
+      final boolean removed = sharedList.remove(bagEntry);
+      if (!removed && !closed) {
          throw new IllegalStateException("Attempt to remove an object from the bag that does not exist");
       }
+      return removed;
    }
 
    /**