Skip to content

[Java] copyStringTensorToArray() leaks tempBuffer on three early-return paths, and does not delete the jstring it creates per element #31705

Description

@K-ANOY

Describe the issue

The string tensor conversion helper allocates tempBuffer up front and has a string_tensor_cleanup: label that frees its native buffers, but three early-return paths bypass that label. The conversion loop also creates one Java string per tensor element without checking the result or deleting the reference.

File: java/src/main/native/OrtJniUtil.c
Function: copyStringTensorToArray

1. tempBuffer leaks on three early returns

size_t bufferSize = 16;
char * tempBuffer = malloc(bufferSize);
if (tempBuffer == NULL) { ... }

// Get the buffer size needed
size_t totalStringLength = 0;
OrtErrorCode code = checkOrtStatus(jniEnv, api, api->GetStringTensorDataLength(tensor, &totalStringLength));
if (code != ORT_OK) {
  return code;                    /* (1) tempBuffer leaked */
}

char * characterBuffer = malloc(sizeof(char)*(totalStringLength+length));
if (characterBuffer == NULL) {
  throwOrtException(jniEnv, 1, "Not enough memory");
  return ORT_FAIL;                /* (2) tempBuffer leaked */
}

size_t * offsets = allocarray(sizeof(size_t), length+1);
if (offsets == NULL) {
  free((void*)characterBuffer);
  throwOrtException(jniEnv, 1, "Not enough memory");
  return ORT_FAIL;                /* (3) tempBuffer leaked */
}

Path (3) is the clearest about the intent: it remembers to free characterBuffer but not tempBuffer. Path (1) matters most in practice, because it is reachable whenever GetStringTensorDataLength() returns a non-OK status — an ordinary runtime error rather than an out-of-memory condition — so this leak does not require memory pressure to trigger.

The function already has the right cleanup path; these three returns simply do not use it. The realloc-failure path a few lines below does the right thing and jumps to string_tensor_cleanup.

2. Temporary jstring references are not deleted, and NewStringUTF() is unchecked

for (size_t i = 0; i < length; i++) {
  ...
  jobject tempString = (*jniEnv)->NewStringUTF(jniEnv,tempBuffer);
  (*jniEnv)->SetObjectArrayElement(jniEnv,outputArray,safecast_size_t_to_jsize(i),tempString);
}

SetObjectArrayElement() stores the string in outputArray but does not consume the local reference. These references are reclaimed when the native method returns, so this is not a leak that persists across calls — the problem is that length is the tensor element count, which is data-dependent and unbounded, so converting one large string tensor grows the local reference table far beyond what the JVM pre-allocates.

The return value of NewStringUTF() is also unchecked; on failure the code continues into SetObjectArrayElement() with a pending exception and keeps iterating.

This is on the inference path rather than a setup path: copyStringTensorToArray() is reached from OnnxTensor.getValue() (via createStringArrayFromTensor) and from OnnxMap key/value extraction.

Expected behavior: every allocation in the function is released on every exit path, as the string_tensor_cleanup: label was written to do; each per-element jstring is checked and released after being stored into the output array.

Actual behavior: tempBuffer is leaked on three of the function's exit paths, one of which is a plain API-failure path; and length local references stay live for the duration of the call.

Suggested fix — route the three early returns through the existing cleanup label so every allocation has one matching free:

OrtErrorCode code = checkOrtStatus(jniEnv, api, api->GetStringTensorDataLength(tensor, &totalStringLength));
if (code != ORT_OK) {
  goto string_tensor_cleanup;
}

The label already null-checks tempBuffer; characterBuffer and offsets would need to be initialised to NULL and null-checked there too. For the loop, check the result, set the element, then delete the reference:

jobject tempString = (*jniEnv)->NewStringUTF(jniEnv, tempBuffer);
if (tempString == NULL) {
  code = ORT_FAIL;                /* exception already pending */
  goto string_tensor_cleanup;
}
(*jniEnv)->SetObjectArrayElement(
    jniEnv, outputArray, safecast_size_t_to_jsize(i), tempString);
(*jniEnv)->DeleteLocalRef(jniEnv, tempString);

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 java/src/main/native/OrtJniUtil.c and find copyStringTensorToArray.
  2. Note tempBuffer is allocated at the top of the function and that the function ends with a string_tensor_cleanup: label which frees it.
  3. Confirm the three return statements after that allocation — the GetStringTensorDataLength() failure, the characterBuffer allocation failure, and the offsets allocation failure — return directly instead of jumping to the label.
  4. In the conversion loop, confirm there is no DeleteLocalRef() for tempString and no null check on NewStringUTF().

Runtime observation (optional):

  1. Run any model whose output is a string tensor, or a ZipMap-style model producing a map output, under valgrind --leak-check=full, with a build where GetStringTensorDataLength() fails; observe the 16-byte tempBuffer block reported as definitely lost.
  2. For the reference growth, run java -Xcheck:jni and call OnnxTensor.getValue() on a string tensor with a large element count; observe the local reference table warning.

Urgency

No response

Platform

Linux

OS Version

Ubuntu 22.04.5 LTS

ONNX Runtime Installation

Built from Source

ONNX Runtime Version or Commit ID

cf5e9eb

ONNX Runtime API

Python

Architecture

X64

Execution Provider

Default CPU

Execution Provider Library Version

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    api:Javaissues related to the Java API

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions