You likely have seen this already, but I stumbled across this looking at the implementation of Bitmap.recycle()

This seems to be how the platform handles a similar pattern:
https://android.googlesource.com/platform/libcore/+/17d9d33/luni/src/main/java/libcore/util/NativeAllocationRegistry.java

Interesting! That uses Cleaner, which is at least similar to what Tommy has implemented (in particular, it's built on ShadowReferences too). Having a general-purpose Cleaner implementation would be pretty neat overall (and maybe it would be a nice design to layer native destruction on top of that).

Yes, jreck@ mentioned the NativeAllocationRegistry, so I did have a look at that. They have a concept of the size of the native object that they inform the runtime about:

  dalvik.system.VMRuntime.getRuntime().registerNativeAllocation((int)Math.min(size, Integer.MAX_VALUE));
  dalvik.system.VMRuntime.getRuntime().registerNativeFree((int)Math.min(size, Integer.MAX_VALUE));

Which is really neat, but I don't think we have access to dalvik directly. It doesn't look like the java.lang.Runtime we have access to gives us the same type of options though.

I definitely considered having a general purpose cleaner, and building the NativeRefAssassin and friends on top of it, but I wasn't sure where we would use that instead of using try-with-resource or invoking destroy() ourselves.

The current CL (https://chromium-review.googlesource.com/c/chromium/src/+/704180) is related to the work for simplifying the JNI layer, and in particular the Callback class, where it would probably not be as easy to require developers to invoke destroy() in Java. In the future, it would be great if anything related to JNI was autogenerated.

After the CL for the NativeRefAssassin and NativeRefHolder is ready, maybe I could look into making a more general purpose solution and move the current stuff over to it.

Yay! A general-purpose cleaner could be useful to implement a CloseGuard that can actually be used outside of the framework, for example. But overall, I agree that explicit cleanup is probably preferred.

Note that the problem of leaking the native object can be solved the other way around: by wrapping native object into NativePtr Java object, and using that object instead of plain long.

I.e. on the Java side we end up with something like:

class MyClass {

  @CalledByNative
  private static MyClass create(NativePtr nativePtr) {
    return new MyClass(nativePtr);
  }

  private final NativePtr mNativePtr;

  private MyClass(NativePtr nativePtr) {
    mNativePtr = nativePtr;
  }

  public void foo() {
    nativeFoo(mNativePtr);
  }

  private static native nativeFoo(mNativePtr);
}

No longs to store pointers, no destroy() methods, etc.


But I've briefly checked our codebase for destroy() methods, and I can see that they're sometimes doing something else in addition to destroying a native object. For example org/chromium/chromoting/jni/Client.java also does disconnectFromHost() and org/chromium/chrome/browser/PasswordUIView.java does mObservers.clear(). Neither NativeRefAssassin nor this NativePtr proposal support such cases.


I think it would be interesting to do a survey of how destroy() methods are used (or more broadly, how native objects are managed by Java code).