From 1f8ec21a44a455ad8821e954dffa2420aa03bff7 Mon Sep 17 00:00:00 2001 From: Sreekanth Vadigi Date: Tue, 16 Jun 2026 09:29:01 +0000 Subject: [PATCH] Fix closeOnCompletion() infinite recursion on close (ES-1978361) ResultSet.close() and Statement.close() recursed into each other indefinitely when closeOnCompletion() was enabled, causing a StackOverflowError / apparent hang after a successful query. Make ResultSet.close() idempotent: return early if already closed, and set isClosed before notifying the parent Statement so the re-entrant close triggered via handleResultSetClose() short-circuits at the guard. Adds regression tests for both entry points (ResultSet.close() and Statement.close()) with closeOnCompletion() enabled. Co-authored-by: Isaac Signed-off-by: Sreekanth Vadigi --- NEXT_CHANGELOG.md | 1 + .../jdbc/api/impl/DatabricksResultSet.java | 11 ++- .../api/impl/DatabricksStatementTest.java | 70 +++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index a972de65e..3de6630e1 100644 --- a/NEXT_CHANGELOG.md +++ b/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. diff --git a/src/main/java/com/databricks/jdbc/api/impl/DatabricksResultSet.java b/src/main/java/com/databricks/jdbc/api/impl/DatabricksResultSet.java index 540699407..32bc29ab4 100644 --- a/src/main/java/com/databricks/jdbc/api/impl/DatabricksResultSet.java +++ b/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(); } diff --git a/src/test/java/com/databricks/jdbc/api/impl/DatabricksStatementTest.java b/src/test/java/com/databricks/jdbc/api/impl/DatabricksStatementTest.java index 73ed7f33c..520de0686 100644 --- a/src/test/java/com/databricks/jdbc/api/impl/DatabricksStatementTest.java +++ b/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()); + } }