microsoft / microsoft/onnxruntime

[Java] String-array input paths never delete the local references returned by GetObjectArrayElement()

Open
#31,707 3 comments 0 reactions 0 assignees View on GitHub
api:Java stale
Dominant language
C++
Stars
21.9k
Forks
4.2k
Avg merge
4d 11h
Merged PRs (30d)
184

Description

### Describe the issue

Several JNI bindings that take `String[]` inputs obtain the elements with `GetObjectArrayElement()`, pin their UTF-8 contents, and later release the UTF-8 buffers — but never delete the temporary local references to the strings themselves. `ReleaseStringUTFChars()` releases the character buffer, not the reference returned by `GetObjectArrayElement()`.

Files:
- `java/src/main/native/ai_onnxruntime_providers_OrtCUDAProviderOptions.c`
- `java/src/main/native/ai_onnxruntime_providers_OrtTensorRTProviderOptions.c`
- `java/src/main/native/ai_onnxruntime_OrtSession_SessionOptions.c`
- `java/src/main/native/ai_onnxruntime_OrtSession.c`
- `java/src/main/native/ai_onnxruntime_OnnxTensor.c`

### Provider options

`applyToNative()` in the CUDA and TensorRT bindings creates two local references per option in the acquire loop and two more per option in the release loop, and deletes none of them:

```c
for (jsize i = 0; i < keyLength; i++) {
jobject key = (*jniEnv)->GetObjectArrayElement(jniEnv, jKeyArr, i);
keys[i] = (*jniEnv)->GetStringUTFChars(jniEnv, key, NULL);
jobject value = (*jniEnv)->GetObjectArrayElement(jniEnv, jValueArr, i);
values[i] = (*jniEnv)->GetStringUTFChars(jniEnv, value, NULL);
}
```

So a call configuring N options leaves 4N string references live for the duration of the native method. (The release loop in these two files also passes the wrong array; I have filed that separately as it is an independent correctness bug.)

### Session options

`SessionOptions.addExecutionProvider(...)` already keeps the `jstring` references in arrays, precisely so the UTF chars can be released against the right objects later:

```c
jkeyArray[i] = (jstring)((*jniEnv)->GetObjectArrayElement(jniEnv, configKeyArr, i));
jvalueArray[i] = (jstring)((*jniEnv)->GetObjectArrayElement(jniEnv, configValueArr, i));
keyArray[i] = (*jniEnv)->GetStringUTFChars(jniEnv, jkeyArray[i], NULL);
valueArray[i] = (*jniEnv)->GetStringUTFChars(jniEnv, jvalueArray[i], NULL);
...
(*jniEnv)->ReleaseStringUTFChars(jniEnv, jkeyArray[i], keyArray[i]);
(*jniEnv)->ReleaseStringUTFChars(jniEnv, jvalueArray[i], valueArray[i]);
```

The UTF chars are released correctly, but the references in `jkeyArray` / `jvalueArray` are never deleted. Both blocks with this shape in the file are affected.

### Session run and tensor creation

`OrtSession.run()` has the same pattern for the input and output name arrays: `javaInputStrings[i]` and `javaOutputStrings[i]` are fetched, used for `GetStringUTFChars()`, and never deleted. `OnnxTensor.createString()` fetches each string-array element twice — once in the acquire loop and once again in the release loop — and deletes neither.

The codebase already applies the correct discipline elsewhere; for example `OnnxTensor.c` deletes the array reference it no longer needs after reading an element out of it:

```c
jobject output = (*jniEnv)->GetObjectArrayElement(jniEnv, outputArray, 0);

// Free array
(*jniEnv)->DeleteLocalRef(jniEnv, outputArray);
```

**Expected behavior:** each local reference obtained from `GetObjectArrayElement()` is deleted once its UTF chars have been released, so the local reference table stays bounded regardless of input size.

**Actual behavior:** the references stay live until the native method returns, and the count is caller-driven in every one of these paths:
- provider options: 4 references per configured option;
- `addExecutionProvider`: 2 per configuration entry;
- `OrtSession.run()`: one per input name plus one per output name, so it scales with the model's I/O count and is hit on every inference call;
- `OnnxTensor.createString()`: 2 per element of the string tensor being created, which is data-dependent and unbounded.

`run()` and `createString()` are the ones that concern me most, since they are on the per-inference path rather than on one-time setup. A large string tensor or a wide model can push the local reference table well past what the JVM pre-allocates, which produces `-Xcheck:jni` warnings and, for large enough inputs, local reference allocation failure.

**Suggested fix** — keep the `jstring` reference that was used for `GetStringUTFChars()`, release the chars against that same reference, then delete it:

```c
jstring key = (jstring)(*jniEnv)->GetObjectArrayElement(jniEnv, jKeyArr, i);
jstring value = (jstring)(*jniEnv)->GetObjectArrayElement(jniEnv, jValueArr, i);
keys[i] = (*jniEnv)->GetStringUTFChars(jniEnv, key, NULL);
values[i] = (*jniEnv)->GetStringUTFChars(jniEnv, value, NULL);
...
(*jniEnv)->ReleaseStringUTFChars(jniEnv, key, keys[i]);
(*jniEnv)->ReleaseStringUTFChars(jniEnv, value, values[i]);
(*jniEnv)->DeleteLocalRef(jniEnv, key);
(*jniEnv)->DeleteLocalRef(jniEnv, value);
```

This also removes the second `GetObjectArrayElement()` call in the release loops, so the acquire and release sides can no longer disagree about which array an element came from. A complete patch should also handle partial failures: `GetObjectArrayElement()` and `GetStringUTFChars()` results are currently unchecked in these loops, and on failure the release loop should only release the entries that were successfully acquired.

### To reproduce

This is a source-level defect, confirmed by reading the code at the commit below. Inference results are unaffected, which is why functional tests do not catch it.

Static confirmation:
1. Open each of the five files listed above.
2. Find every `GetObjectArrayElement()` call whose result is passed to `GetStringUTFChars()`.
3. Confirm that the matching `ReleaseStringUTFChars()` exists but that no `DeleteLocalRef()` is ever called on the `jstring` itself.
4. Compare against the `DeleteLocalRef(jniEnv, outputArray)` in `ai_onnxruntime_OnnxTensor.c`, which shows the intended discipline.

### Urgency

_No response_

### Platform

Linux

### OS Version

Ubuntu 22.04.5 LTS

### ONNX Runtime Installation

Built from Source

### ONNX Runtime Version or Commit ID

cf5e9eba49b07b4b4ab71ed7903cded9f1446690

### ONNX Runtime API

Python

### Architecture

X64

### Execution Provider

Default CPU

### Execution Provider Library Version

_No response_

Contributor guide

Open the contributing guide

Research direction

Inspect GetObjectArrayElement() and GetStringUTFChars() usage in the five named JNI files, starting with the provider-option applyToNative() paths and the SessionOptions, OrtSession.run(), and OnnxTensor.createString() entry points. Ensure each acquired jstring reference is retained through UTF cleanup and deleted afterward, avoid the second fetch in release loops, and handle partially acquired entries safely.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, java
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.