Skip to content

JaCoP JNI wrapper stores FindClass local refs instead of global refs #282

Description

@QiuYucheng2003

Summary

The JaCoP JNI helpers keep FindClass results as JNI local references on a thread that creates the JVM and stays attached. Locals are not released when these C++ functions return.

The same header already provides GlobalRef / NewGlobalRef, but class cache and several call sites do not use them.

1. Class::DoInit caches a local jclass

mp/solvers/jacop/java.h

Lines 259 to 262 in d75ef49

void DoInit(Env env) {
class_ = env.FindClass(Info::name());
ctor_ = env.GetMethod(class_, "<init>", Info::ctor_sig());
}

void DoInit(Env env) {
  class_ = env.FindClass(Info::name());
  ctor_ = env.GetMethod(class_, "<init>", Info::ctor_sig());
}

class_ is a long-lived member. A local reference is only valid on the creating thread and only while that thread remains attached. It should be promoted with NewGlobalRef (or stored in GlobalRef) and deleted with DeleteGlobalRef in the destructor.

This also occupies a local-reference table slot for the life of the Class object.

2. Env::NewObject(class_name, ...) drops the class local ref

mp/solvers/jacop/java.cc

Lines 142 to 149 in d75ef49

jobject Env::NewObject(const char *class_name, const char *ctor_sig, ...) {
jclass cls = FindClass(class_name);
jmethodID ctor = GetMethod(cls, "<init>", ctor_sig);
std::va_list args;
va_start(args, ctor_sig);
jobject result = env_->NewObjectV(cls, ctor, args);
va_end(args);
return Check(result, "NewObjectV");

jclass cls = FindClass(class_name);
jmethodID ctor = GetMethod(cls, "<init>", ctor_sig);
jobject result = env_->NewObjectV(cls, ctor, args);
return Check(result, "NewObjectV");

cls is never DeleteLocalRef'd. The returned jobject is an ownership transfer and is fine. Each call through this overload leaves one extra jclass local on the attached thread.

3. Env::Throw leaves two locals

mp/solvers/jacop/java.cc

Lines 124 to 131 in d75ef49

void Env::Throw(jthrowable exception, const char *method_name) {
env_->ExceptionClear();
jmethodID getMessage = GetMethod(FindClass("java/lang/Object"),
"toString", "()Ljava/lang/String;");
String message(env_, static_cast<jstring>(Check(
env_->CallObjectMethod(exception, getMessage), "CallObjectMethod")));
throw JavaError(fmt::format(
"{} failed: {}", method_name, message.c_str()), exception);

  • The temporary from FindClass("java/lang/Object") is not deleted.
  • String only calls ReleaseStringUTFChars in its destructor; it does not DeleteLocalRef the jstring from toString().

JavaError also stores the jthrowable from ExceptionOccurred(), which is still a local reference.

This path is not a hot loop; the class-cache and NewObject(class_name, ...) issues matter more.

Suggested fix

  • In DoInit, store NewGlobalRef(FindClass(...)) (or a GlobalRef member). Delete it in ~ClassBase.
  • After GetMethod in Env::NewObject(const char *, ...), DeleteLocalRef(cls).
  • In Throw, keep the FindClass result in a variable, wrap/delete the toString jstring, and convert the throwable with NewGlobalRef if it must outlive the JNI call.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions