This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 7e47d0d7d refactor(java/driver/jni): guard against null handles (#4426)
7e47d0d7d is described below
commit 7e47d0d7d105a38fb5800b6159b7498a83786f63
Author: David Li <[email protected]>
AuthorDate: Tue Jun 23 15:37:38 2026 -0700
refactor(java/driver/jni): guard against null handles (#4426)
Assisted-by: GPT-5.5 <[email protected]>
---
java/driver/jni/pom.xml | 1 +
java/driver/jni/src/main/cpp/jni_wrapper.cc | 2 +-
.../driver/jni/impl/NativeConnectionHandle.java | 6 ++-
.../adbc/driver/jni/impl/NativeDatabaseHandle.java | 6 ++-
.../arrow/adbc/driver/jni/impl/NativeHandle.java | 1 +
.../driver/jni/impl/NativeStatementHandle.java | 6 ++-
.../arrow/adbc/driver/jni/JniDriverTest.java | 20 +++++++++
.../adbc/driver/jni/impl/NativeHandleTest.java | 51 ++++++++++++++++++++++
8 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/java/driver/jni/pom.xml b/java/driver/jni/pom.xml
index dc7f91ebf..9f2df211d 100644
--- a/java/driver/jni/pom.xml
+++ b/java/driver/jni/pom.xml
@@ -88,6 +88,7 @@
<systemPropertyVariables>
<arrow.test.dataRoot>${project.basedir}/../../../testing/data</arrow.test.dataRoot>
</systemPropertyVariables>
+ <argLine>${surefireArgLine} -Xcheck:jni
--add-opens=java.base/java.nio=ALL-UNNAMED</argLine>
</configuration>
</plugin>
</plugins>
diff --git a/java/driver/jni/src/main/cpp/jni_wrapper.cc
b/java/driver/jni/src/main/cpp/jni_wrapper.cc
index 6221a60cb..56e5e4718 100644
--- a/java/driver/jni/src/main/cpp/jni_wrapper.cc
+++ b/java/driver/jni/src/main/cpp/jni_wrapper.cc
@@ -639,7 +639,7 @@
Java_org_apache_arrow_adbc_driver_jni_impl_NativeAdbc_statementExecutePartitions
env->SetByteArrayRegion(partition, 0, static_cast<jsize>(length),
reinterpret_cast<const
jbyte*>(partitions.partitions[i]));
if (env->ExceptionCheck()) goto cleanupall;
- env->CallObjectMethod(result, native_result_add_partition, partition);
+ env->CallVoidMethod(result, native_result_add_partition, partition);
if (env->ExceptionCheck()) goto cleanupall;
// The Java side has a reference now, so free the per-iteration local
// reference to avoid overflowing the local reference table
diff --git
a/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeConnectionHandle.java
b/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeConnectionHandle.java
index 7ad3efec8..2955ed6c7 100644
---
a/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeConnectionHandle.java
+++
b/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeConnectionHandle.java
@@ -23,7 +23,11 @@ public class NativeConnectionHandle extends NativeHandle {
}
long getConnectionHandle() {
- return state.nativeHandle;
+ long handle = state.nativeHandle;
+ if (handle == 0) {
+ throw new IllegalStateException("Native connection handle is closed");
+ }
+ return handle;
}
@Override
diff --git
a/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeDatabaseHandle.java
b/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeDatabaseHandle.java
index 45c7aa376..b21b600e5 100644
---
a/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeDatabaseHandle.java
+++
b/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeDatabaseHandle.java
@@ -23,7 +23,11 @@ public class NativeDatabaseHandle extends NativeHandle {
}
long getDatabaseHandle() {
- return state.nativeHandle;
+ long handle = state.nativeHandle;
+ if (handle == 0) {
+ throw new IllegalStateException("Native database handle is closed");
+ }
+ return handle;
}
@Override
diff --git
a/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeHandle.java
b/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeHandle.java
index a2975faad..c78056f08 100644
---
a/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeHandle.java
+++
b/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeHandle.java
@@ -57,6 +57,7 @@ abstract class NativeHandle implements AutoCloseable {
try {
closer.close(handle);
} catch (AdbcException e) {
+ // If we fail to close, the handle will leak, but there's not much we
can do anyways
throw new RuntimeException(e);
}
}
diff --git
a/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeStatementHandle.java
b/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeStatementHandle.java
index cbc2a563f..3d6a549d3 100644
---
a/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeStatementHandle.java
+++
b/java/driver/jni/src/main/java/org/apache/arrow/adbc/driver/jni/impl/NativeStatementHandle.java
@@ -23,7 +23,11 @@ public class NativeStatementHandle extends NativeHandle {
}
long getStatementHandle() {
- return state.nativeHandle;
+ long handle = state.nativeHandle;
+ if (handle == 0) {
+ throw new IllegalStateException("Native statement handle is closed");
+ }
+ return handle;
}
@Override
diff --git
a/java/driver/jni/src/test/java/org/apache/arrow/adbc/driver/jni/JniDriverTest.java
b/java/driver/jni/src/test/java/org/apache/arrow/adbc/driver/jni/JniDriverTest.java
index 7415cce23..14adf62e0 100644
---
a/java/driver/jni/src/test/java/org/apache/arrow/adbc/driver/jni/JniDriverTest.java
+++
b/java/driver/jni/src/test/java/org/apache/arrow/adbc/driver/jni/JniDriverTest.java
@@ -18,6 +18,7 @@
package org.apache.arrow.adbc.driver.jni;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.io.File;
@@ -143,6 +144,25 @@ class JniDriverTest {
}
}
+ @Test
+ void statementThrowsAfterClose() throws Exception {
+ try (final BufferAllocator allocator = new RootAllocator()) {
+ JniDriver driver = new JniDriver(allocator);
+ Map<String, Object> parameters = new HashMap<>();
+ JniDriver.PARAM_DRIVER.set(parameters, "adbc_driver_sqlite");
+
+ try (final AdbcDatabase db = driver.open(parameters);
+ final AdbcConnection conn = db.connect()) {
+ final AdbcStatement stmt = conn.createStatement();
+ stmt.close();
+
+ assertThatThrownBy(() -> stmt.setSqlQuery("SELECT 1"))
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Native statement handle is closed");
+ }
+ }
+ }
+
@Test
void queryLarge() throws Exception {
try (final BufferAllocator allocator = new RootAllocator()) {
diff --git
a/java/driver/jni/src/test/java/org/apache/arrow/adbc/driver/jni/impl/NativeHandleTest.java
b/java/driver/jni/src/test/java/org/apache/arrow/adbc/driver/jni/impl/NativeHandleTest.java
new file mode 100644
index 000000000..1329fc840
--- /dev/null
+++
b/java/driver/jni/src/test/java/org/apache/arrow/adbc/driver/jni/impl/NativeHandleTest.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.adbc.driver.jni.impl;
+
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.jupiter.api.Test;
+
+class NativeHandleTest {
+ @Test
+ void databaseHandleThrowsWhenClosed() {
+ NativeDatabaseHandle handle = new NativeDatabaseHandle(0);
+
+ assertThatThrownBy(handle::getDatabaseHandle)
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Native database handle is closed");
+ }
+
+ @Test
+ void connectionHandleThrowsWhenClosed() {
+ NativeConnectionHandle handle = new NativeConnectionHandle(0);
+
+ assertThatThrownBy(handle::getConnectionHandle)
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Native connection handle is closed");
+ }
+
+ @Test
+ void statementHandleThrowsWhenClosed() {
+ NativeStatementHandle handle = new NativeStatementHandle(0);
+
+ assertThatThrownBy(handle::getStatementHandle)
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessage("Native statement handle is closed");
+ }
+}
