#31 - datasource.offline() closes busy connections ... should let them close on return

Author: rbygrave

src/main/java/io/ebean/datasource/pool/ConnectionPool.java

@@ -788,16 +788,20 @@ public void shutdown() {
    */
   @Override
   public void shutdown(boolean deregisterDriver) {
-    offline();
+    shutdownPool(true);
     if (deregisterDriver) {
       deregisterDriver();
     }
   }
 
   @Override
   public void offline() {
+    shutdownPool(false);
+  }
+
+  private void shutdownPool(boolean closeBusyConnections) {
     stopHeartBeatIfRunning();
-    queue.shutdown();
+    queue.shutdown(closeBusyConnections);
     dataSourceUp.set(false);
   }
 

src/main/java/io/ebean/datasource/pool/PooledConnectionQueue.java

@@ -311,7 +311,7 @@ private PooledConnection _getPooledConnectionWaitLoop() throws SQLException, Int
     }
   }
 
-  public void shutdown() {
+  void shutdown(boolean closeBusyConnections) {
     lock.lock();
     try {
       doingShutdown = true;
@@ -320,10 +320,15 @@ public void shutdown() {
 
       closeFreeConnections(true);
 
-      if (!busyList.isEmpty()) {
-        logger.warn("Closing busy connections on shutdown size: " + busyList.size());
-        dumpBusyConnectionInformation();
-        closeBusyConnections(0);
+      if (!closeBusyConnections) {
+        // connections close on return to pool
+        lastResetTime = System.currentTimeMillis() - 100;
+      } else {
+        if (!busyList.isEmpty()) {
+          logger.warn("Closing busy connections on shutdown size: " + busyList.size());
+          dumpBusyConnectionInformation();
+          closeBusyConnections(0);
+        }
       }
     } finally {
       lock.unlock();

src/test/java/io/ebean/datasource/pool/ConnectionPoolOfflineTest.java

@@ -6,6 +6,7 @@
 import org.testng.annotations.Test;
 
 import java.sql.Connection;
+import java.sql.PreparedStatement;
 import java.sql.SQLException;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -89,6 +90,48 @@ public void offlineOffline() {
     assertThat(pool.isOnline()).isFalse();
   }
 
+  @Test
+  public void offline_whenBusy_allowed() throws SQLException, InterruptedException {
+
+    DataSourceConfig config = config().setUrl("jdbc:h2:mem:offlineWhenBusy");
+
+    ConnectionPool pool = new ConnectionPool("offlineWhenBusy", config);
+    pool.online();
+
+    final Connection busy = pool.getConnection();
+    Thread thread = new Thread(() -> {
+      try {
+        System.out.println("busy connection being used");
+        try (PreparedStatement statement = busy.prepareStatement("select 'hello' from dual")) {
+          statement.execute();
+        }
+        Thread.sleep(3000);
+        System.out.println("busy connection closing now");
+        busy.close();
+      } catch (SQLException | InterruptedException e) {
+        e.printStackTrace();
+        throw new RuntimeException("Should not fail!!");
+      }
+    });
+
+    thread.start();
+
+    Thread.sleep(200);
+    System.out.println("-- taking pool offline (with a busy connection)");
+    assertEquals(1, pool.getStatus(false).getBusy());
+
+    pool.offline();
+    assertEquals(0, pool.getStatus(false).getFree());
+    assertEquals(1, pool.getStatus(false).getBusy()); // still 1 busy connection
+
+    // a bit of time to let busy connection finish and close
+    Thread.sleep(4000);
+
+    // all done now
+    assertEquals(0, pool.getStatus(false).getFree());
+    assertEquals(0, pool.getStatus(false).getBusy());
+  }
+
   @Test
   public void offlineGetConnection_expect_goesOnline() throws SQLException {