fix: Actually close unclosed results. Previously was not closing the first unclosed result fixes #1903 (#1905)

davecramer · davecramer · commit fc25aebcec1b · 2021-02-17T15:15:38.000-05:00
* fix: Actually close unclosed results. Previously was not closing the first unclosed result fixes #1903 * rename and comment closing of unclosed results to make it more clear
diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java
@@ -332,7 +332,11 @@ public boolean executeWithFlags(int flags) throws SQLException {
         PSQLState.WRONG_OBJECT_TYPE);
   }
 
-  private void closeUnclosedResults() throws SQLException {
+  /*
+  If there are multiple result sets we close any that have been processed and left open
+  by the client.
+   */
+  private void closeUnclosedProcessedResults() throws SQLException {
     synchronized (this) {
       ResultWrapper resultWrapper = this.firstUnclosedResult;
       ResultWrapper currentResult = this.result;
@@ -354,7 +358,11 @@ protected void closeForNextExecution() throws SQLException {
 
     // Close any existing resultsets associated with this statement.
     synchronized (this) {
-      closeUnclosedResults();
+      closeUnclosedProcessedResults();
+
+      if ( this.result != null && this.result.getResultSet() != null ) {
+        this.result.getResultSet().close();
+      }
       result = null;
 
       ResultWrapper generatedKeys = this.generatedKeys;
@@ -1188,7 +1196,7 @@ public boolean getMoreResults(int current) throws SQLException {
       // CLOSE_ALL_RESULTS
       if (current == Statement.CLOSE_ALL_RESULTS) {
         // Close preceding resultsets.
-        closeUnclosedResults();
+        closeUnclosedProcessedResults();
       }
 
       // Done.
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/StatementTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/StatementTest.java
@@ -98,6 +98,14 @@ public void testClose() throws SQLException {
     }
   }
 
+  @Test
+  public void testResultSetClosed() throws SQLException {
+    Statement stmt = con.createStatement();
+    ResultSet rs = stmt.executeQuery("select 1");
+    stmt.close();
+    assertTrue(rs.isClosed());
+  }
+
   /**
    * Closing a Statement twice is not an error.
    */
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc4/jdbc41/CloseOnCompletionTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc4/jdbc41/CloseOnCompletionTest.java
@@ -5,6 +5,7 @@
 
 package org.postgresql.test.jdbc4.jdbc41;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -94,8 +95,28 @@ public void testNoResultSet() throws SQLException {
   @Test
   public void testExecuteTwice() throws SQLException {
     PreparedStatement s = conn.prepareStatement("SELECT 1");
-    s.closeOnCompletion();
+
+    s.executeQuery();
     s.executeQuery();
+
+  }
+
+  @Test
+  public void testCloseOnCompletionExecuteTwice() throws SQLException {
+    PreparedStatement s = conn.prepareStatement("SELECT 1");
+
+    /*
+     once we set close on completion we should only be able to execute one as the second execution
+     will close the resultsets from the first one which will close the statement.
+     */
+
+    s.closeOnCompletion();
     s.executeQuery();
+    try {
+      s.executeQuery();
+    } catch (SQLException ex) {
+      assertEquals(ex.getMessage(),"This statement has been closed.");
+    }
+
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -98,6 +98,14 @@ public void testClose() throws SQLException {`
`98`	`98`	`}`
`99`	`99`	`}`
`100`	`100`
	`101`	`+ @Test`
	`102`	`+ public void testResultSetClosed() throws SQLException {`
	`103`	`+ Statement stmt = con.createStatement();`
	`104`	`+ ResultSet rs = stmt.executeQuery("select 1");`
	`105`	`+ stmt.close();`
	`106`	`+ assertTrue(rs.isClosed());`
	`107`	`+ }`
	`108`	`+`
`101`	`109`	`/**`
`102`	`110`	`* Closing a Statement twice is not an error.`
`103`	`111`	`*/`