fix(java/driver/jni): check for pending exceptions more thoroughly - #4398
Conversation
There was a problem hiding this comment.
Pull request overview
This PR strengthens JNI error handling in the Java JNI driver by explicitly checking for pending Java exceptions after specific JNI calls, and returning early to let the original Java exception propagate instead of continuing native work (which could trigger additional failures or secondary exceptions).
Changes:
- Added
env->ExceptionCheck()short-circuit returns after array length and element access inopenDatabaseand metadata helpers. - Added exception checks around byte-array allocation and population (
NewByteArray,SetByteArrayRegion) in*GetOptionBytespaths. - Added exception checks after reading array contents (
GetArrayLength,Get*ArrayRegion) in*SetOptionBytespaths.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
After JNI calls that can leave a pending Java exception (GetArrayLength, GetObjectArrayElement, GetIntArrayRegion, GetByteArrayRegion, NewByteArray, SetByteArrayRegion) and are followed by further JNI or native ADBC work, check env->ExceptionCheck() and return the function's existing error default, letting the pending exception propagate to Java rather than performing more work or raising a second exception. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
d5367cc to
d7dcc4c
Compare
zeroshade
left a comment
zeroshade
left a comment
There was a problem hiding this comment.
In general looks good to me, but I don't know JNI behavior well enough so I have a single question.
| jclass nativeHandleKlass = RequireImplClass(env, "NativeDatabaseHandle"); | ||
| jmethodID nativeHandleCtor = RequireMethod(env, nativeHandleKlass, "<init>", "(J)V"); | ||
| const jsize num_params = env->GetArrayLength(parameters); | ||
| if (env->ExceptionCheck()) return nullptr; |
There was a problem hiding this comment.
returning null is the correct behavior as opposed to throwing the exception or otherwise explicitly propagating it?
There was a problem hiding this comment.
The exception is already thrown on the Java side. Throwing a C++ exception is unrelated to what happens in Java.
After JNI calls that can leave a pending Java exception (GetArrayLength, GetObjectArrayElement, GetIntArrayRegion, GetByteArrayRegion, NewByteArray, SetByteArrayRegion) and are followed by further JNI or native ADBC work, check env->ExceptionCheck() and return the function's existing error default, letting the pending exception propagate to Java rather than performing more work or raising a second exception.
Generated-by: Claude Opus 4.8 noreply@anthropic.com