Memory leaks in methods looking up Class objects

[dmitry-timofeev]

Overview

JNIEnv methods accepting Desc<JClass> leak a local reference to the Class object if they create a new local reference.

Description

If an implementation of Desc#lookup that creates a new local reference is passed to the JNIEnv methods, the created local reference will be leaked (resulting in a leak of native JVM heap and a (potential) leak of referenced Java object). E.g.,

env.call_static_method("java/lang/Math", "abs", "(I)I", &[JValue::from(-1 as jint)])

will leak a single local reference to an instance of Class<java.lang.Math> created here each time it is called.

There are about two dozens of Desc#lookups in the codebase.

Which client code is NOT affected (benign leaks)

Such leaks can be considered benign if all created local references are freed later:

• Refs created in native methods called from Java — freed by the JVM when the native method returns.
• Refs created in a native method attached briefly to the JVM — freed after the thread is detached.
• Refs created in a lambda passed to with_local_frame — freed after the last statement in that lambda.

Which client code IS affected

Code using affected methods in:

• Native threads attached to the JVM for a long or indefinite time. Although such code shall generally use with_local_frame/auto_local to manage the references created by the user, it must be noted that it's not required if it performs only simple calls that do not expose any references that have to be deleted. In other words, it is reasonable to expect that the library methods do not leak — and that's the specified behaviour of the C APIs:

To ensure that programmers can manually free local references, JNI functions are not allowed to create extra local references, except for references they return as the result.

• Long-running loops.

How to reproduce

See #108

Possible solutions

1. Always perform lookups in a local frame to ensure we delete refs if we create them. Slight performance regression — need to evaluate.
2. Have methods with distinct signatures that perform lookups and delete manually in them. A certain UX regression.
3. Distinguish (?) somehow when we do create a reference during lookup:
   • By using a combination of new_local_ref + AutoLocal. If a lookup is performed — the result is simply wrapped in an auto_local. If an existing reference is returned (e.g., JClass -> JClass), it is first duplicated with new_local_ref to keep the original intact, and then wrapped in auto_local. Slight performance regression in case the existing reference is used — unneeded new_local_ref + delete_local_ref
   • By returning a structure similar to auto_local that remembers if it shall delete the local reference (or retrofitting such functionality in AutoLocal). No performance hit when it is not needed, but higher complexity
4. (?)

See Also

• Call delete_local_ref for classes objects #40
• new_object: accessed stale local reference #44

[koutheir]

Which client code is NOT affected (benign leaks)

Such leaks can be considered benign if all created local references are freed later:

* Refs created in native methods called from Java — freed by the JVM when the native method returns.

If the native method implementation deals with large arrays of objects, then it might run out of local references storage sooner.

To ensure that programmers can manually free local references, JNI functions are not allowed to create extra local references, except for references they return as the result.

From a library user perspective, it is reasonable to assume this.

1. Always perform lookups in a local frame to ensure we delete refs _if_ we create them. _Likely to be a performance regression — need to evaluate._

Also likely to consume more memory due to the additional local references stacks created.