tensorflow / tensorflow/tflite-support

Unchecked `GetStringUTFChars()` result and local-reference accumulation in the tokenizer JNI helpers

Open
#1,014 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
441
Forks
146
PR merge metrics
No merged PRs in 30d

Description

File: tensorflow_lite_support/cc/text/tokenizers/tokenizer_jni_lib.cc

Functions:

  • nativeTokenize
  • nativeConvertTokensToIds
1. nativeConvertTokensToIds dereferences an unchecked GetStringUTFChars() result
for (int i = 0; i < count; i++) {
  auto jstr =
      reinterpret_cast<jstring>(env->GetObjectArrayElement(jtokens, i));
  const char* token = env->GetStringUTFChars(jstr, JNI_FALSE);
  int id;
  tokenizer->LookupId(token, &id);
  jid_ptr[i] = id;
  env->ReleaseStringUTFChars(jstr, token);
}

GetStringUTFChars() returns NULL and raises OutOfMemoryError if it cannot allocate the copy. The result goes straight into LookupId(), which will dereference it. The loop also never calls ExceptionCheck(), so it keeps calling JNI functions with a pending exception for the remaining iterations.

The second argument is also wrong: GetStringUTFChars() takes a jboolean* isCopy out-parameter, and this passes the value JNI_FALSE. It happens to work because JNI_FALSE is 0 and therefore a null pointer constant, but the intent should be nullptr.

The same function does not check NewIntArray() or GetIntArrayElements() either.

2. Temporary string references are not deleted

nativeTokenize() creates one jstring per subword and stores it in the result array:

for (int i = 0; i < subwords.size(); ++i) {
  jstring text = CheckNotNull(env, env->NewStringUTF(subwords[i].data()));
  if (env->ExceptionCheck()) {
    return nullptr;
  }

  env->SetObjectArrayElement(result, i, text);
}

SetObjectArrayElement() does not consume the local reference. The same applies to jstr in nativeConvertTokensToIds()ReleaseStringUTFChars() releases the character buffer, not the reference returned by GetObjectArrayElement().

These references are reclaimed when the native method returns, so this is not a leak across calls. The reason it still matters is that both loop bounds are input-driven: the subword count for a long input text, and the token count in the caller's array. A single tokenization of a large document can therefore create thousands of live references, which grows the local reference table and shows up under -Xcheck:jni.

nativeTokenize() additionally leaks the FindClass("java/lang/String") reference, though only once per call.

Suggested fix

For the unchecked pointer:

const char* token = env->GetStringUTFChars(jstr, nullptr);
if (token == nullptr) {
  env->DeleteLocalRef(jstr);
  env->ReleaseIntArrayElements(result, jid_ptr, 0);
  return nullptr;               // exception already pending
}

For the references:

env->SetObjectArrayElement(result, i, text);
env->DeleteLocalRef(text);

and:

env->ReleaseStringUTFChars(jstr, token);
env->DeleteLocalRef(jstr);

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in tensorflow_lite_support/cc/text/tokenizers/tokenizer_jni_lib.cc and inspect nativeTokenize and nativeConvertTokensToIds, including each JNI allocation and loop. Verify the JNI error paths handle pending exceptions and array access failures, and that temporary references are released; use -Xcheck:jni to confirm the local-reference accumulation is gone.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
mobile-dev
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.