Fix closeOnCompletion() infinite recursion on ResultSet/Statement close (ES-1978361)

NEXT_CHANGELOG.md

@@ -7,6 +7,7 @@
 ### Updated
 
 ### Fixed
+- Fixed `StackOverflowError` / hang when closing a `ResultSet` or `Statement` with `closeOnCompletion()` enabled.
 - Fixed `setCatalog()` and `setSchema()` producing invalid SQL (e.g. `SET CATALOG ``name``) when the catalog or schema name was passed already wrapped in backticks. Backticks are now stripped before wrapping, and `getCatalog()`/`getSchema()` return the bare identifier name.
 - Fixed metadata SQL generation for catalog, schema, and table identifiers containing backticks.
 - Fixed SEA result truncation when direct results are disabled. Large, highly-compressible results that span multiple chunks were delivered inline via the old hybrid path and truncated to the first chunk. The SQL Execution path now uses an async (`0s`) wait timeout when direct results are disabled, so results are returned via external links and fetched in full.

src/main/java/com/databricks/jdbc/api/impl/DatabricksResultSet.java

@@ -360,10 +360,19 @@ public boolean next() throws SQLException {
 
   @Override
   public void close() throws DatabricksSQLException {
+    // Idempotency guard. Besides making close() safe to call more than once, this also breaks the
+    // mutual recursion that occurs when closeOnCompletion() is enabled (ES-1978361): closing the
+    // ResultSet notifies the parent Statement (handleResultSetClose), which closes the Statement,
+    // which closes the ResultSet again. Returning early on the re-entrant call stops the loop.
+    if (isClosed) {
+      return;
+    }
+    // Mark closed before notifying the parent Statement, so any re-entrant close() triggered via
+    // handleResultSetClose() short-circuits at the guard above instead of recursing.
+    isClosed = true;
     stopHeartbeat();
     // Proactively close server operation when ResultSet is closed explicitly.
     closeServerOperation();
-    isClosed = true;
     if (executionResult != null) {
       executionResult.close();
     }

src/test/java/com/databricks/jdbc/api/impl/DatabricksStatementTest.java

@@ -2188,4 +2188,74 @@ public void testStatementClose_noDoubleRpc_whenResultSetNotClosed() throws Excep
     // Should fire exactly one closeStatement RPC, not two
     verify(client, times(1)).closeStatement(STATEMENT_ID);
   }
+
+  /**
+   * Regression test for ES-1978361. With closeOnCompletion() enabled, closing the ResultSet must
+   * not recurse infinitely between ResultSet.close() and Statement.close() (previously a
+   * StackOverflowError). Exercises the ResultSet.close() entry point with a real ResultSet wired to
+   * a real Statement.
+   */
+  @Test
+  public void testCloseOnCompletion_resultSetCloseDoesNotRecurse() throws Exception {
+    IDatabricksConnectionContext connectionContext =
+        DatabricksConnectionContext.parse(JDBC_URL, new Properties());
+    DatabricksConnection connection = new DatabricksConnection(connectionContext, client);
+    DatabricksStatement statement = new DatabricksStatement(connection);
+
+    IExecutionResult execResult = mock(IExecutionResult.class);
+    DatabricksResultSetMetaData rsMeta = mock(DatabricksResultSetMetaData.class);
+    DatabricksResultSet resultSet =
+        new DatabricksResultSet(
+            new StatementStatus().setState(StatementState.SUCCEEDED),
+            STATEMENT_ID,
+            StatementType.QUERY,
+            statement,
+            execResult,
+            rsMeta,
+            false);
+    statement.setStatementId(STATEMENT_ID);
+    statement.resultSet = resultSet;
+    statement.closeOnCompletion();
+    assertTrue(statement.isCloseOnCompletion());
+
+    // Closing the ResultSet auto-closes the Statement (closeOnCompletion). Must not StackOverflow.
+    assertDoesNotThrow(() -> resultSet.close());
+
+    assertTrue(resultSet.isClosed());
+    assertTrue(statement.isClosed());
+    // Server operation must be closed at most once despite the close path being entered twice.
+    verify(client, atMost(1)).closeStatement(STATEMENT_ID);
+  }
+
+  /**
+   * Regression test for ES-1978361, statement-close entry point. Closing the Statement (which
+   * closes its ResultSet, which calls back via closeOnCompletion) must not recurse infinitely.
+   */
+  @Test
+  public void testCloseOnCompletion_statementCloseDoesNotRecurse() throws Exception {
+    IDatabricksConnectionContext connectionContext =
+        DatabricksConnectionContext.parse(JDBC_URL, new Properties());
+    DatabricksConnection connection = new DatabricksConnection(connectionContext, client);
+    DatabricksStatement statement = new DatabricksStatement(connection);
+
+    IExecutionResult execResult = mock(IExecutionResult.class);
+    DatabricksResultSetMetaData rsMeta = mock(DatabricksResultSetMetaData.class);
+    DatabricksResultSet resultSet =
+        new DatabricksResultSet(
+            new StatementStatus().setState(StatementState.SUCCEEDED),
+            STATEMENT_ID,
+            StatementType.QUERY,
+            statement,
+            execResult,
+            rsMeta,
+            false);
+    statement.setStatementId(STATEMENT_ID);
+    statement.resultSet = resultSet;
+    statement.closeOnCompletion();
+
+    assertDoesNotThrow(() -> statement.close());
+
+    assertTrue(statement.isClosed());
+    assertTrue(resultSet.isClosed());
+  }
 }