FIX: connection leak in query-iterator (#1164)

src/main/java/io/ebeaninternal/server/query/CQuery.java

@@ -36,6 +36,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.NoSuchElementException;
 
 /**
  * An object that represents a SqlSelect statement.
@@ -421,6 +422,7 @@ public class CQuery<T> implements DbReadContext, CancelableQuery {
 
     if (!moveToNextRow()) {
       if (currentBean == null) {
+        nextBean = null;
         return false;
       } else {
         // the last bean
@@ -508,6 +510,9 @@ public class CQuery<T> implements DbReadContext, CancelableQuery {
       auditIterateNextBean();
     }
     hasNextCache = false;
+    if (nextBean == null) {
+      throw new NoSuchElementException();
+    }
     return nextBean;
   }
 

src/main/java/io/ebeaninternal/server/query/CQueryIteratorSimple.java

@@ -14,6 +14,8 @@ class CQueryIteratorSimple<T> implements QueryIterator<T> {
   private final CQuery<T> cquery;
 
   private final OrmQueryRequest<T> request;
+  
+  private boolean closed;
 
   CQueryIteratorSimple(CQuery<T> cquery, OrmQueryRequest<T> request) {
     this.cquery = cquery;
@@ -22,11 +24,17 @@ class CQueryIteratorSimple<T> implements QueryIterator<T> {
 
   @Override
   public boolean hasNext() {
+    boolean ret = false;
     try {
       request.flushPersistenceContextOnIterate();
-      return cquery.hasNext();
+      ret = cquery.hasNext();
+      return ret;
     } catch (SQLException e) {
       throw cquery.createPersistenceException(e);
+    } finally {
+      if (!ret) {
+        close();
+      }
     }
   }
 
@@ -38,9 +46,12 @@ class CQueryIteratorSimple<T> implements QueryIterator<T> {
 
   @Override
   public void close() {
-    cquery.updateExecutionStatisticsIterator();
-    cquery.close();
-    request.endTransIfRequired();
+    if (!closed) {
+      closed = true;
+      cquery.updateExecutionStatisticsIterator();
+      cquery.close();
+      request.endTransIfRequired();
+    }
   }
 
   @Override

src/test/java/org/tests/query/TestQueryFindIterate.java

@@ -9,16 +9,17 @@ import org.tests.model.basic.Customer;
 import org.tests.model.basic.Order;
 import org.tests.model.basic.OrderShipment;
 import org.tests.model.basic.ResetBasicData;
+import org.avaje.datasource.DataSourcePool;
 import org.ebeantest.LoggedSqlCollector;
 import org.junit.Test;
 
 import javax.persistence.PersistenceException;
 import java.util.List;
+import java.util.NoSuchElementException;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 public class TestQueryFindIterate extends BaseTestCase {
 
@@ -224,4 +225,39 @@ public class TestQueryFindIterate extends BaseTestCase {
       }
     });
   }
+  
+  @Test
+  public void testCloseConnection() throws Exception {
+    ResetBasicData.reset();
+    DataSourcePool dsPool = (DataSourcePool) server().getPluginApi().getDataSource();
+    int startConns = dsPool.getStatus(false).getBusy();
+    QueryIterator<Customer> queryIterator = server().find(Customer.class)
+        .where()
+        .isNotNull("name")
+        .setMaxRows(3)
+        .order().asc("id")
+        .findIterate();
+
+    assertThat(dsPool.getStatus(false).getBusy()).isEqualTo(startConns + 1);
+
+    assertTrue(queryIterator.hasNext());
+    assertThat(queryIterator.next()).isNotNull();
+    assertThat(dsPool.getStatus(false).getBusy()).isEqualTo(startConns + 1);
+
+    assertTrue(queryIterator.hasNext());
+    assertThat(queryIterator.next()).isNotNull();
+    assertThat(dsPool.getStatus(false).getBusy()).isEqualTo(startConns + 1);
+
+    assertTrue(queryIterator.hasNext());
+    assertThat(queryIterator.next()).isNotNull();
+    assertThat(dsPool.getStatus(false).getBusy()).isEqualTo(startConns + 1);
+
+    assertFalse(queryIterator.hasNext());
+    assertThat(dsPool.getStatus(false).getBusy()).isEqualTo(startConns);
+    try  {
+      queryIterator.next();
+      fail("noSuchElementException expected");
+    } catch (NoSuchElementException e) {}
+
+  }
 }