Skip to content

GH-1385: Match the JNI return type when closing exported C Data - #1390

Open
hutiefang76 wants to merge 2 commits into
apache:mainfrom
hutiefang76:frank/arrow1385-jni-void-release-20261024
Open

hutiefang76 wants to merge 2 commits into
apache:mainfrom
hutiefang76:frank/arrow1385-jni-void-release-20261024

Conversation

@hutiefang76

@hutiefang76 hutiefang76 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

What's Changed

The C Data release callbacks invoke PrivateData.close() through CallObjectMethod, although its JNI descriptor is ()V. With OpenJ9's JNI checks enabled, releasing an exported array, schema or stream can abort the JVM. Use CallVoidMethod in both callbacks.

The existing C Data tests already exercise the affected callbacks, so this PR adds no new test suite.

Validation:

  • Linux aarch64, IBM Semeru OpenJ9 21.0.12.1, -Xcheck:jni: the existing RoundtripTest#testSchema and StreamTest#getSchemaError each abort with JVMJNCK046E against the unchanged JNI library (exit 87, at releaseSchema and releaseArrayStream respectively). Both pass against the fixed library without JNI check errors.
  • macOS arm64, JDK 21: affected C Data reactor clean verify passed: 110 C Data cases, no failures or skips; vector and memory dependencies passed as well.

Closes #1385.

AI-generated PR description. Investigation, implementation and automated review assisted by Codex and DeepSeek.

Use CallVoidMethod for PrivateData.close in array/schema and stream
release callbacks. Keep cleanup intact and exercise all three exported
structures, including close failures and nested retained buffers.

Generated-by: DeepSeek V4 Flash and Codex
@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is all of this really necessary? It's not already tested?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’re right, the existing tests cover the callbacks needed for this fix. I checked RoundtripTest#testSchema and StreamTest#getSchemaError separately on OpenJ9 with -Xcheck:jni: both abort with the original JNI library and pass with the two-line fix. I’ve removed ReleaseTest; the PR now changes only the two JNI calls. The affected reactor also passes clean verify with the existing 110 C Data tests.

AI-generated response, checked against the executed test results.

@lidavidm lidavidm added the bug-fix PRs that fix a big. label Oct 24, 2026
@github-actions github-actions Bot added this to the 20.0.0 milestone Oct 24, 2026
Remove the duplicate ReleaseTest suite after confirming the existing schema and stream tests each reproduce the JNI return-type error on OpenJ9 and pass with CallVoidMethod.

Generated-by: Codex and DeepSeek V4 Flash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PRs that fix a big.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C Data] JNI release callback calls void PrivateData.close() with CallObjectMethod

2 participants