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
|
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
|
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
|
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.
Summary
The JaCoP JNI helpers keep
FindClassresults 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::DoInitcaches a localjclassmp/solvers/jacop/java.h
Lines 259 to 262 in d75ef49
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 withNewGlobalRef(or stored inGlobalRef) and deleted withDeleteGlobalRefin the destructor.This also occupies a local-reference table slot for the life of the
Classobject.2.
Env::NewObject(class_name, ...)drops the class local refmp/solvers/jacop/java.cc
Lines 142 to 149 in d75ef49
clsis neverDeleteLocalRef'd. The returnedjobjectis an ownership transfer and is fine. Each call through this overload leaves one extrajclasslocal on the attached thread.3.
Env::Throwleaves two localsmp/solvers/jacop/java.cc
Lines 124 to 131 in d75ef49
FindClass("java/lang/Object")is not deleted.Stringonly callsReleaseStringUTFCharsin its destructor; it does notDeleteLocalRefthejstringfromtoString().JavaErroralso stores thejthrowablefromExceptionOccurred(), 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
DoInit, storeNewGlobalRef(FindClass(...))(or aGlobalRefmember). Delete it in~ClassBase.GetMethodinEnv::NewObject(const char *, ...),DeleteLocalRef(cls).Throw, keep theFindClassresult in a variable, wrap/delete thetoStringjstring, and convert the throwable withNewGlobalRefif it must outlive the JNI call.