This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-java.git
The following commit(s) were added to refs/heads/main by this push:
new 4b2d53ed1 GH-1205: Fix byte-array element leak in FromSchemaByteArray
(#1249)
4b2d53ed1 is described below
commit 4b2d53ed1639173b30aa0dc694ce3a0fa9965bb6
Author: Prateek Ganigi <[email protected]>
AuthorDate: Tue Oct 6 01:14:40 2026 -0700
GH-1205: Fix byte-array element leak in FromSchemaByteArray (#1249)
## What's Changed
`FromSchemaByteArray()` in `dataset/src/main/cpp/jni_util.cc` acquires
the Java byte-array elements via `GetByteArrayElements()`, but the
matching `ReleaseByteArrayElements()` was only reached on the success
path. When `arrow::ipc::ReadSchema()` failed, `ARROW_ASSIGN_OR_RAISE`
returned early and skipped the release, leaking the pinned/copied array
buffer on every call with malformed or incompatible serialized schema
bytes (e.g. via `createDataset()`).
This releases the elements through an RAII guard so they are always
freed on every exit path - success or error, and stays correct if
additional early returns are added later.
Closes #1205.
---
dataset/src/main/cpp/jni_util.cc | 12 ++--
.../arrow/dataset/jni/TestFromSchemaByteArray.java | 79 ++++++++++++++++++++++
2 files changed, 87 insertions(+), 4 deletions(-)
diff --git a/dataset/src/main/cpp/jni_util.cc b/dataset/src/main/cpp/jni_util.cc
index 35bfb328f..23c9db06c 100644
--- a/dataset/src/main/cpp/jni_util.cc
+++ b/dataset/src/main/cpp/jni_util.cc
@@ -352,13 +352,17 @@ arrow::Result<std::shared_ptr<arrow::Schema>>
FromSchemaByteArray(
arrow::ipc::DictionaryMemo in_memo;
int schemaBytes_len = env->GetArrayLength(schemaBytes);
jbyte* schemaBytes_data = env->GetByteArrayElements(schemaBytes, nullptr);
+ // Ensure the pinned/copied Java array elements are always released, even
when
+ // schema parsing fails and an error Result is returned early below.
+ auto release_elements = [&](jbyte* data) {
+ env->ReleaseByteArrayElements(schemaBytes, data, JNI_ABORT);
+ };
+ std::unique_ptr<jbyte, decltype(release_elements)>
elements_guard(schemaBytes_data,
+
release_elements);
auto serialized_schema = std::make_shared<arrow::Buffer>(
reinterpret_cast<uint8_t*>(schemaBytes_data), schemaBytes_len);
arrow::io::BufferReader buf_reader(serialized_schema);
- ARROW_ASSIGN_OR_RAISE(std::shared_ptr<arrow::Schema> schema,
- arrow::ipc::ReadSchema(&buf_reader, &in_memo))
- env->ReleaseByteArrayElements(schemaBytes, schemaBytes_data, JNI_ABORT);
- return schema;
+ return arrow::ipc::ReadSchema(&buf_reader, &in_memo);
}
arrow::Status ExportRecordBatch(JNIEnv* env, const
std::shared_ptr<RecordBatch>& batch,
jlong struct_array) {
diff --git
a/dataset/src/test/java/org/apache/arrow/dataset/jni/TestFromSchemaByteArray.java
b/dataset/src/test/java/org/apache/arrow/dataset/jni/TestFromSchemaByteArray.java
new file mode 100644
index 000000000..238678498
--- /dev/null
+++
b/dataset/src/test/java/org/apache/arrow/dataset/jni/TestFromSchemaByteArray.java
@@ -0,0 +1,79 @@
+/*
+ * 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.dataset.jni;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.File;
+import java.lang.reflect.Field;
+import org.apache.arrow.dataset.ParquetWriteSupport;
+import org.apache.arrow.dataset.file.FileFormat;
+import org.apache.arrow.dataset.file.FileSystemDatasetFactory;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Regression test for the native {@code FromSchemaByteArray} helper
(GH-1205). Passing malformed
+ * serialized schema bytes to {@code JniWrapper#createDataset} exercises the
former leak branch in
+ * {@code FromSchemaByteArray} (where {@code ReleaseByteArrayElements} was
skipped on error) and
+ * asserts it fails gracefully with a Java exception rather than crashing.
+ *
+ * <p>This guards the error path; it does not directly assert that the native
byte-array elements
+ * were released. The leaked bytes are a JVM-internal copy made by {@code
GetByteArrayElements},
+ * which is not tracked by {@code NativeMemoryPool} nor observable through any
portable Java API, so
+ * a deterministic leak assertion isn't feasible here. See the PR #1249
discussion for details.
+ */
+public class TestFromSchemaByteArray extends TestNativeDataset {
+
+ @TempDir public File TMP;
+
+ public static final String AVRO_SCHEMA_USER = "user.avsc";
+
+ private static long factoryId(NativeDatasetFactory factory) throws Exception
{
+ Field field =
NativeDatasetFactory.class.getDeclaredField("datasetFactoryId");
+ field.setAccessible(true);
+ return field.getLong(factory);
+ }
+
+ @Test
+ public void testCreateDatasetWithMalformedSchemaBytes() throws Exception {
+ ParquetWriteSupport writeSupport =
+ ParquetWriteSupport.writeTempFile(AVRO_SCHEMA_USER, TMP, 1, "a");
+ FileSystemDatasetFactory factory =
+ new FileSystemDatasetFactory(
+ rootAllocator(),
+ NativeMemoryPool.getDefault(),
+ FileFormat.PARQUET,
+ writeSupport.getOutputURI());
+ try {
+ final long datasetFactoryId = factoryId(factory);
+ // Bytes that are not a valid serialized Arrow schema, so native
ReadSchema fails and the
+ // error path of FromSchemaByteArray is taken.
+ final byte[] malformedSchemaBytes = new byte[] {0, 1, 2, 3, 4, 5, 6, 7};
+
+ // Repeat many times to keep hitting the error path that previously
skipped the element
+ // release; each iteration must fail gracefully rather than crash.
+ for (int i = 0; i < 1000; i++) {
+ assertThrows(
+ RuntimeException.class,
+ () -> JniWrapper.get().createDataset(datasetFactoryId,
malformedSchemaBytes));
+ }
+ } finally {
+ factory.close();
+ }
+ }
+}
