Fix #128 fix accounting issue with totalConnections when aborting con…

…nections during shutdown.
brettwooldridge · Jul 31, 2014 · 281c287 · 281c287
1 parent 6b7143f
commit 281c287
Show file tree

Hide file tree

Showing 6 changed files with 372 additions and 316 deletions.
diff --git a/hikaricp-java6/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/hikaricp-java6/src/main/java/com/zaxxer/hikari/pool/HikariPool.java
@@ -484,6 +484,7 @@ private void abortActiveConnections() throws InterruptedException
       for (IHikariConnectionProxy connectionProxy : connectionBag.values(STATE_IN_USE)) {
          try {
             connectionProxy.abort(assassinExecutor);
+            totalConnections.decrementAndGet();
          }
          catch (AbstractMethodError e) {
             quietlyCloseConnection(connectionProxy);
@@ -492,7 +493,6 @@ private void abortActiveConnections() throws InterruptedException
             quietlyCloseConnection(connectionProxy);
          }
          finally {
-            totalConnections.decrementAndGet();
             try {
                connectionBag.remove(connectionProxy);
             }

diff --git a/hikaricp-java6/src/test/java/com/zaxxer/hikari/ShutdownTest.java b/hikaricp-java6/src/test/java/com/zaxxer/hikari/ShutdownTest.java
@@ -16,6 +16,7 @@
 
 package com.zaxxer.hikari;
 
+import java.sql.Connection;
 import java.sql.SQLException;
 import java.util.concurrent.TimeUnit;
 
@@ -33,161 +34,187 @@
  */
 public class ShutdownTest
 {
-    @Before
-    public void beforeTest()
-    {
-        StubConnection.count.set(0);
-    }
-
-    @After
-    public void afterTest()
-    {
-        StubConnection.slowCreate = false;
-    }
-
-    @Test
-    public void testShutdown1() throws SQLException
-    {
-        Assert.assertSame("StubConnection count not as expected", 0, StubConnection.count.get());
-
-        StubConnection.slowCreate = true;
-
-        HikariConfig config = new HikariConfig();
-        config.setMinimumIdle(0);
-        config.setMaximumPoolSize(10);
-        config.setInitializationFailFast(true);
-        config.setConnectionTestQuery("VALUES 1");
-        config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
-
-        final HikariDataSource ds = new HikariDataSource(config);
-        HikariPool pool = TestElf.getPool(ds);
-
-        Thread[] threads = new Thread[10];
-        for (int i = 0; i < 10; i++)
-        {
-            threads[i] = new Thread() {
-                public void run() {
-                    try
-                    {
-                        if (ds.getConnection() != null)
-                        {
-                            PoolUtilities.quietlySleep(TimeUnit.SECONDS.toMillis(1));
-                        }
-                    }
-                    catch (SQLException e)
-                    {
-                    }
-                }
-            };
-            threads[i].setDaemon(true);
-            threads[i].start();
-        }
-
-        PoolUtilities.quietlySleep(300);
-
-        Assert.assertTrue("Totals 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());
-    }
-
-    @Test
-    public void testShutdown2() throws SQLException
-    {
-        Assert.assertSame("StubConnection count not as expected", 0, StubConnection.count.get());
-
-        StubConnection.slowCreate = true;
-
-        HikariConfig config = new HikariConfig();
-        config.setMinimumIdle(10);
-        config.setMaximumPoolSize(10);
-        config.setInitializationFailFast(false);
-        config.setConnectionTestQuery("VALUES 1");
-        config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
-
-        HikariDataSource ds = new HikariDataSource(config);
-        HikariPool pool = TestElf.getPool(ds);
-
-        PoolUtilities.quietlySleep(300);
-
-        Assert.assertTrue("Totals 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());
-    }
-
-    @Test
-    public void testShutdown3() throws SQLException
-    {
-        Assert.assertSame("StubConnection count not as expected", 0, StubConnection.count.get());
-
-        StubConnection.slowCreate = true;
-
-        HikariConfig config = new HikariConfig();
-        config.setMinimumIdle(5);
-        config.setMaximumPoolSize(5);
-        config.setInitializationFailFast(true);
-        config.setConnectionTestQuery("VALUES 1");
-        config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
-
-        HikariDataSource ds = new HikariDataSource(config);
-        HikariPool pool = TestElf.getPool(ds);
-
-        PoolUtilities.quietlySleep(300);
-
-        Assert.assertTrue("Totals 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());
-    }
-
-    @Test
-    public void testShutdown4() throws SQLException
-    {
-        StubConnection.slowCreate = true;
-
-        HikariConfig config = new HikariConfig();
-        config.setMinimumIdle(10);
-        config.setMaximumPoolSize(10);
-        config.setInitializationFailFast(false);
-        config.setConnectionTestQuery("VALUES 1");
-        config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
-
-        HikariDataSource ds = new HikariDataSource(config);
-
-        PoolUtilities.quietlySleep(300);
-
-        ds.close();
-
-        long start = System.currentTimeMillis();
-        while (PoolUtilities.elapsedTimeMs(start) < TimeUnit.SECONDS.toMillis(5) && threadCount() > 0)
-        {
-            PoolUtilities.quietlySleep(250);
-        }
-
-        Assert.assertSame("Thread was leaked", 0, threadCount());
-    }
-
-	private int threadCount()
-	{
-	    Thread[] threads = new Thread[Thread.activeCount() * 2];
-	    Thread.enumerate(threads);
-
-	    int count = 0;
-	    for (Thread thread : threads)
-	    {
-	        count += (thread != null && thread.getName().startsWith("Hikari")) ? 1 : 0;
-	    }
-
-	    return count;
-	}
+   @Before
+   public void beforeTest()
+   {
+      StubConnection.count.set(0);
+   }
+
+   @After
+   public void afterTest()
+   {
+      StubConnection.slowCreate = false;
+   }
+
+   @Test
+   public void testShutdown1() throws SQLException
+   {
+      Assert.assertSame("StubConnection count not as expected", 0, StubConnection.count.get());
+
+      StubConnection.slowCreate = true;
+
+      HikariConfig config = new HikariConfig();
+      config.setMinimumIdle(0);
+      config.setMaximumPoolSize(10);
+      config.setInitializationFailFast(true);
+      config.setConnectionTestQuery("VALUES 1");
+      config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
+
+      final HikariDataSource ds = new HikariDataSource(config);
+      HikariPool pool = TestElf.getPool(ds);
+
+      Thread[] threads = new Thread[10];
+      for (int i = 0; i < 10; i++) {
+         threads[i] = new Thread() {
+            public void run()
+            {
+               try {
+                  if (ds.getConnection() != null) {
+                     PoolUtilities.quietlySleep(TimeUnit.SECONDS.toMillis(1));
+                  }
+               }
+               catch (SQLException e) {
+               }
+            }
+         };
+         threads[i].setDaemon(true);
+         threads[i].start();
+      }
+
+      PoolUtilities.quietlySleep(300);
+
+      Assert.assertTrue("Totals 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());
+   }
+
+   @Test
+   public void testShutdown2() throws SQLException
+   {
+      Assert.assertSame("StubConnection count not as expected", 0, StubConnection.count.get());
+
+      StubConnection.slowCreate = true;
+
+      HikariConfig config = new HikariConfig();
+      config.setMinimumIdle(10);
+      config.setMaximumPoolSize(10);
+      config.setInitializationFailFast(false);
+      config.setConnectionTestQuery("VALUES 1");
+      config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
+
+      HikariDataSource ds = new HikariDataSource(config);
+      HikariPool pool = TestElf.getPool(ds);
+
+      PoolUtilities.quietlySleep(300);
+
+      Assert.assertTrue("Totals 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());
+   }
+
+   @Test
+   public void testShutdown3() throws SQLException
+   {
+      Assert.assertSame("StubConnection count not as expected", 0, StubConnection.count.get());
+
+      StubConnection.slowCreate = true;
+
+      HikariConfig config = new HikariConfig();
+      config.setMinimumIdle(5);
+      config.setMaximumPoolSize(5);
+      config.setInitializationFailFast(true);
+      config.setConnectionTestQuery("VALUES 1");
+      config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
+
+      HikariDataSource ds = new HikariDataSource(config);
+      HikariPool pool = TestElf.getPool(ds);
+
+      PoolUtilities.quietlySleep(300);
+
+      Assert.assertTrue("Totals 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());
+   }
+
+   @Test
+   public void testShutdown4() throws SQLException
+   {
+      StubConnection.slowCreate = true;
+
+      HikariConfig config = new HikariConfig();
+      config.setMinimumIdle(10);
+      config.setMaximumPoolSize(10);
+      config.setInitializationFailFast(false);
+      config.setConnectionTestQuery("VALUES 1");
+      config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
+
+      HikariDataSource ds = new HikariDataSource(config);
+
+      PoolUtilities.quietlySleep(300);
+
+      ds.close();
+
+      long start = System.currentTimeMillis();
+      while (PoolUtilities.elapsedTimeMs(start) < TimeUnit.SECONDS.toMillis(5) && threadCount() > 0) {
+         PoolUtilities.quietlySleep(250);
+      }
+
+      Assert.assertSame("Thread was leaked", 0, threadCount());
+   }
+
+   @Test
+   public void testShutdown5() throws SQLException
+   {
+      Assert.assertSame("StubConnection count not as expected", 0, StubConnection.count.get());
+
+      StubConnection.slowCreate = false;
+
+      HikariConfig config = new HikariConfig();
+      config.setMinimumIdle(5);
+      config.setMaximumPoolSize(5);
+      config.setInitializationFailFast(true);
+      config.setConnectionTestQuery("VALUES 1");
+      config.setDataSourceClassName("com.zaxxer.hikari.mocks.StubDataSource");
+
+      HikariDataSource ds = new HikariDataSource(config);
+      HikariPool pool = TestElf.getPool(ds);
+
+      Connection[] connections = new Connection[5];
+      for (int i = 0; i < 5; i++) {
+         connections[i] = ds.getConnection();
+      }
+
+      Assert.assertTrue("Totals 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());
+   }
+
+   private int threadCount()
+   {
+      Thread[] threads = new Thread[Thread.activeCount() * 2];
+      Thread.enumerate(threads);
+
+      int count = 0;
+      for (Thread thread : threads) {
+         count += (thread != null && thread.getName().startsWith("Hikari")) ? 1 : 0;
+      }
+
+      return count;
+   }
 }
diff --git a/hikaricp-java6/src/test/java/com/zaxxer/hikari/mocks/StubConnection.java b/hikaricp-java6/src/test/java/com/zaxxer/hikari/mocks/StubConnection.java
@@ -402,6 +402,7 @@ public String getSchema() throws SQLException
     /** {@inheritDoc} */
     public void abort(Executor executor) throws SQLException
     {
+       throw new SQLException("Intentianal exception during abort");
     }
 
     /** {@inheritDoc} */

diff --git a/hikaricp/src/main/java/com/zaxxer/hikari/pool/HikariPool.java b/hikaricp/src/main/java/com/zaxxer/hikari/pool/HikariPool.java
@@ -473,12 +473,12 @@ private void abortActiveConnections() throws InterruptedException
       connectionBag.values(STATE_IN_USE).parallelStream().forEach(connectionProxy -> {
          try {
             connectionProxy.abort(assassinExecutor);
+            totalConnections.decrementAndGet();
          }
          catch (SQLException | AbstractMethodError e) {
             quietlyCloseConnection(connectionProxy);
          }
          finally {
-            totalConnections.decrementAndGet();
             try {
                connectionBag.remove(connectionProxy);
             }