tweag/inline-java

Don't rely on optimizations to avoid reference leaks.

facundominguez opened this issue · 2 comments

The jvm package relies on the compiler to float code like:

unsafePerformIO $ findClass "java/lang/String" >>= newGlobalRefNonFinalized

so the class is effectively created only once.

If the optimization does not happen, a global reference is created every time the control flow passes over the expression.

Some solutions:
1 Collect the global references in an environment and run java calls on a monad which carries this environment and reuses the references when they have been already created.
2. Collect the global references in a global and mutable table where java calls can look them up.
3. Create the global references in global and inmutable variables. These are defined as methods of a class KnownReferenceType:

class IsReferenceType ty => KnownReferenceType (ty :: JType) where
  findTaggedClass :: Tagged ty JClass
  -- can define a default instance here
  1. Use a compiler plugin to generate the instances of KnownReferenceType. We would examine the code and whenever we find that KnownReferenceType ty is needed, we generate an instance for it, provided that ty is enough instantiated to know which class is needed. We would have to investigate whether the plugin interface allows to do this.

We have tried (3). It requires the programmer to declare multiple instances: one instance every time a method of a class is going to be called. It solves the problem and helps finding typos, but it would be better not to have to write the instances.

There are some more fixes.

  1. Don't cache class references.
  2. Release class references as soon as they aren't needed. Therefore, even if optimizations don't cause them to be cached, they will be released.

(6) deserves some more explanation. The following code never releases the global reference

unsafePerformIO $ findClass "java/lang/String" >>= newGlobalRefNonFinalized

We could change it to

unsafePerformIO $ findClass "java/lang/String" >>= newGlobalRef

which releases the global reference when no longer needed. The problem with this variant is that sometimes MethodIDs are cached

unsafePerformIO $ getStaticMethodID klass "..." "..."

and the class reference needs to be kept alive while the MethodID is reachable.

Possibly the jvm package could implement some variant of MethodID that keeps a reference to the class.

In the current code this is fixed by not caching class references ((5) above), only MethodIDs are cached. https://github.com/tweag/inline-java/blob/6dbd549/jvm/src/common/Language/Java/Unsafe.hs#L592-L597

In theory, the JVM could unload the class and invalidate the MethodID, but we haven't experience this in practice.

In any case, the original issue has been addressed.