K-ANOY opened a new issue, #1256:
URL: https://github.com/apache/arrow-java/issues/1256
### Describe the bug, including details regarding any error messages,
version, and platform.
There is a JNI local-reference issue in the dataset helpers that convert
Java `String[]` arrays into C++ containers. They get each element with
`GetObjectArrayElement()` and convert it, but never delete the resulting local
reference.
Files:
- `dataset/src/main/cpp/jni_util.cc`
- `dataset/src/main/cpp/jni_wrapper.cc`
Functions:
- `ToStringVector` (`jni_util.cc`)
- `ToStringMap`, `LoadNamedTables` (`jni_wrapper.cc`)
Relevant code in `ToStringVector()`:
```cpp
std::vector<std::string> ToStringVector(JNIEnv* env, jobjectArray&
str_array) {
int length = env->GetArrayLength(str_array);
std::vector<std::string> vector;
for (int i = 0; i < length; i++) {
auto string =
reinterpret_cast<jstring>(env->GetObjectArrayElement(str_array, i));
vector.push_back(JStringToCString(env, string));
}
return vector;
}
```
`JStringToCString()` does correctly release the native UTF chars it acquires:
```cpp
const char* chars = env->GetStringUTFChars(string, nullptr);
std::string ret(chars);
env->ReleaseStringUTFChars(string, chars);
return ret;
```
but that release only matches `GetStringUTFChars()`. It does not delete the
`jstring` local reference that `GetObjectArrayElement()` returned, so one
reference remains live per element until the native method returns.
`ToStringMap()` has the same pattern with two references per iteration:
```cpp
for (int i = 0; i < length; i += 2) {
auto key = reinterpret_cast<jstring>(env->GetObjectArrayElement(str_array,
i));
auto value =
reinterpret_cast<jstring>(env->GetObjectArrayElement(str_array, i + 1));
map[JStringToCString(env, key)] = JStringToCString(env, value);
}
```
`LoadNamedTables()` also takes two per iteration, and has three `JniThrow()`
exits inside the loop body — the odd-length check and the two `std::stol`
failure handlers — so on those paths the references acquired in the current
iteration are abandoned mid-loop.
These are local references, so they are reclaimed when the native method
returns, and the inputs are option maps, partition columns, and named-table
lists — typically tens of entries. There is no observable leak or reachable
failure here; the reference count is simply higher than necessary for the
duration of the call.
Suggested fix:
```cpp
auto string =
reinterpret_cast<jstring>(env->GetObjectArrayElement(str_array, i));
vector.push_back(JStringToCString(env, string));
env->DeleteLocalRef(string);
```
For the key/value loops, delete both `key` and `value`, including on the
`JniThrow()` paths in `LoadNamedTables()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
