Skip to content

Commit 4f3c8e4

Browse files
committed
GH-1205: Clarify TestFromSchemaByteArray guards the error path, not the leak
1 parent 8667b46 commit 4f3c8e4

1 file changed

Lines changed: 10 additions & 5 deletions

File tree

dataset/src/test/java/org/apache/arrow/dataset/jni/TestFromSchemaByteArray.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,14 @@
2828

2929
/**
3030
* Regression test for the native {@code FromSchemaByteArray} helper (GH-1205). Passing malformed
31-
* serialized schema bytes to {@code JniWrapper#createDataset} must surface a Java exception rather
32-
* than crash, and repeated failures must not leak the pinned/copied Java byte-array elements
33-
* acquired via {@code GetByteArrayElements}.
31+
* serialized schema bytes to {@code JniWrapper#createDataset} exercises the former leak branch in
32+
* {@code FromSchemaByteArray} (where {@code ReleaseByteArrayElements} was skipped on error) and
33+
* asserts it fails gracefully with a Java exception rather than crashing.
34+
*
35+
* <p>This guards the error path; it does not directly assert that the native byte-array elements
36+
* were released. The leaked bytes are a JVM-internal copy made by {@code GetByteArrayElements},
37+
* which is not tracked by {@code NativeMemoryPool} nor observable through any portable Java API, so
38+
* a deterministic leak assertion isn't feasible here. See the PR #1249 discussion for details.
3439
*/
3540
public class TestFromSchemaByteArray extends TestNativeDataset {
3641

@@ -60,8 +65,8 @@ public void testCreateDatasetWithMalformedSchemaBytes() throws Exception {
6065
// error path of FromSchemaByteArray is taken.
6166
final byte[] malformedSchemaBytes = new byte[] {0, 1, 2, 3, 4, 5, 6, 7};
6267

63-
// Repeat many times: before the fix each failed call leaked the acquired array elements.
64-
// The loop keeps the test meaningful as a leak regression while asserting graceful failure.
68+
// Repeat many times to keep hitting the error path that previously skipped the element
69+
// release; each iteration must fail gracefully rather than crash.
6570
for (int i = 0; i < 1000; i++) {
6671
assertThrows(
6772
RuntimeException.class,

0 commit comments

Comments
 (0)