From 3c2c0044817dc3af9693b58fd0844496d4e3620a Mon Sep 17 00:00:00 2001 From: Pedro Matias Date: Sun, 7 Dec 2025 17:42:40 +0000 Subject: [PATCH 1/4] Fix memory leak --- .../apache/arrow/driver/jdbc/ArrowFlightConnection.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java b/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java index f6f17770f1..cf4426e474 100644 --- a/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java +++ b/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java @@ -20,6 +20,7 @@ import io.netty.util.concurrent.DefaultThreadFactory; import java.sql.SQLException; +import java.util.List; import java.util.Properties; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -180,6 +181,12 @@ public Properties getClientInfo() { @Override public void close() throws SQLException { + // Clean up any open Statements + try { + AutoCloseables.close(List.copyOf(statementMap.values())); + } catch (final Exception e) { + throw AvaticaConnection.HELPER.createException(e.getMessage(), e); + } clientHandler.close(); if (executorService != null) { executorService.shutdown(); From 2018ac0febae34cab698ffca76be7331cf388422 Mon Sep 17 00:00:00 2001 From: Pedro Matias Date: Sun, 14 Dec 2025 00:27:27 +0000 Subject: [PATCH 2/4] Add test --- .../arrow/driver/jdbc/ConnectionTest.java | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java index 72e4b222a3..f1af2732f4 100644 --- a/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java +++ b/flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ConnectionTest.java @@ -16,6 +16,7 @@ */ package org.apache.arrow.driver.jdbc; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -26,6 +27,7 @@ import java.sql.Driver; import java.sql.DriverManager; import java.sql.SQLException; +import java.sql.Statement; import java.util.Properties; import org.apache.arrow.driver.jdbc.authentication.UserPasswordAuthentication; import org.apache.arrow.driver.jdbc.client.ArrowFlightSqlClientHandler; @@ -622,4 +624,40 @@ public void testJdbcDriverVersionIntegration() throws Exception { "Expected: " + expectedUserAgent + " but found: " + actualUserAgent); } } + + @Test + public void testStatementsClosedOnConnectionClose() throws Exception { + // create a connection + final Properties properties = new Properties(); + properties.put(ArrowFlightConnectionProperty.HOST.camelName(), "localhost"); + properties.put( + ArrowFlightConnectionProperty.PORT.camelName(), FLIGHT_SERVER_TEST_EXTENSION.getPort()); + properties.put(ArrowFlightConnectionProperty.USER.camelName(), userTest); + properties.put(ArrowFlightConnectionProperty.PASSWORD.camelName(), passTest); + properties.put("useEncryption", false); + + Connection connection = + DriverManager.getConnection( + "jdbc:arrow-flight-sql://" + + FLIGHT_SERVER_TEST_EXTENSION.getHost() + + ":" + + FLIGHT_SERVER_TEST_EXTENSION.getPort(), + properties); + + // create some statements + int numStatements = 3; + Statement[] statements = new Statement[numStatements]; + for (int i = 0; i < numStatements; i++) { + statements[i] = connection.createStatement(); + assertFalse(statements[i].isClosed()); + } + + // close the connection + connection.close(); + + // assert the statements are closed + for (int i = 0; i < numStatements; i++) { + assertTrue(statements[i].isClosed()); + } + } } From 87ee7bfe547e7de5df852166be470600db9c1145 Mon Sep 17 00:00:00 2001 From: Pedro Matias Date: Sun, 14 Dec 2025 00:28:03 +0000 Subject: [PATCH 3/4] Refactor ArrowFlightConnection.close() --- .../driver/jdbc/ArrowFlightConnection.java | 46 +++++++++++++++---- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java b/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java index cf4426e474..3519df9264 100644 --- a/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java +++ b/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java @@ -181,25 +181,55 @@ public Properties getClientInfo() { @Override public void close() throws SQLException { - // Clean up any open Statements + Exception topLevelException = null; try { AutoCloseables.close(List.copyOf(statementMap.values())); } catch (final Exception e) { - throw AvaticaConnection.HELPER.createException(e.getMessage(), e); + topLevelException = e; + } + try { + AutoCloseables.close(clientHandler); + } catch (final Exception e) { + if (topLevelException == null) { + topLevelException = e; + } else { + topLevelException.addSuppressed(e); + } } - clientHandler.close(); - if (executorService != null) { - executorService.shutdown(); + try { + if (executorService != null) { + executorService.shutdown(); + } + } catch (final Exception e) { + if (topLevelException == null) { + topLevelException = e; + } else { + topLevelException.addSuppressed(e); + } } try { - AutoCloseables.close(clientHandler); allocator.getChildAllocators().forEach(AutoCloseables::closeNoChecked); AutoCloseables.close(allocator); - + } catch (final Exception e) { + if (topLevelException == null) { + topLevelException = e; + } else { + topLevelException.addSuppressed(e); + } + } + try { super.close(); } catch (final Exception e) { - throw AvaticaConnection.HELPER.createException(e.getMessage(), e); + if (topLevelException == null) { + topLevelException = e; + } else { + topLevelException.addSuppressed(e); + } + } + if (topLevelException != null) { + throw AvaticaConnection.HELPER.createException( + topLevelException.getMessage(), topLevelException); } } From 5eb999852b3942269615c9717ca214650d2d9891 Mon Sep 17 00:00:00 2001 From: Pedro Matias Date: Sun, 14 Dec 2025 18:01:51 +0000 Subject: [PATCH 4/4] Simplify Connection.close() logic --- .../driver/jdbc/ArrowFlightConnection.java | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java b/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java index 3519df9264..e0f8aa7380 100644 --- a/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java +++ b/flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightConnection.java @@ -189,36 +189,11 @@ public void close() throws SQLException { } try { AutoCloseables.close(clientHandler); - } catch (final Exception e) { - if (topLevelException == null) { - topLevelException = e; - } else { - topLevelException.addSuppressed(e); - } - } - try { if (executorService != null) { executorService.shutdown(); } - } catch (final Exception e) { - if (topLevelException == null) { - topLevelException = e; - } else { - topLevelException.addSuppressed(e); - } - } - - try { allocator.getChildAllocators().forEach(AutoCloseables::closeNoChecked); AutoCloseables.close(allocator); - } catch (final Exception e) { - if (topLevelException == null) { - topLevelException = e; - } else { - topLevelException.addSuppressed(e); - } - } - try { super.close(); } catch (final Exception e) { if (topLevelException == null) {