8

As Hans Boehm in the Google I/O '17 talk "How to Manage Native C++ Memory in Android" suggests I use the PhantomReferenceclass to ensure native peers are deleted properly.

In the linked video at 18 min 57 sec he shows an example implementation of an object registering itself to the PhantomReference class for it's type. This PhantomReference class, he shows then at 19 min 49 sec. So I copied his approach for my example object. See below.

While this approach works fine, it does not scale. I will need to create quite some amount of objects and I haven't found a way to create a base class (either for my objects or a PhantomReference base class) which would take any objects and would handle the native deletion properly.

How can I make a generic base PhantomReference class which can call the native static method on the provided object?

I've tried to transform the PhantomReference generic but the native static deletion method hinders an implementation.

My WorkViewModel

import android.databinding.*;

public class WorkViewModel extends BaseObservable
{
  private long _nativeHandle;

  public WorkViewModel(Database database, int workId)
  {
    _nativeHandle = create(database.getNativeHandle(), workId);
    WorkViewModelPhantomReference.register(this, _nativeHandle);
  }

  private static native long create(long databaseHandle, int workId);
  static native void delete(long nativeHandle);

  @Bindable
  public native int getWorkId();
  public native void setWorkId(int workId);
}

My WorkViewModelPhantomReference

import java.lang.ref.*;
import java.util.*;

public class WorkViewModelPhantomReference extends PhantomReference<WorkViewModel>
{
  private static Set<WorkViewModelPhantomReference> phantomReferences = new HashSet<WorkViewModelPhantomReference>();
  private static ReferenceQueue<WorkViewModel> garbageCollectedObjectsQueue = new ReferenceQueue<WorkViewModel>();
  private long _nativeHandle;

  private WorkViewModelPhantomReference(WorkViewModel workViewModel, long nativeHandle)
  {
    super(workViewModel, garbageCollectedObjectsQueue);
    _nativeHandle = nativeHandle;
  }

  public static void register(WorkViewModel workViewModel, long nativeHandle)
  {
    phantomReferences.add(new WorkViewModelPhantomReference(workViewModel, nativeHandle));
  }

  public static void deleteOrphanedNativePeerObjects()
  {
    WorkViewModelPhantomReference reference;

    while((reference = (WorkViewModelPhantomReference)garbageCollectedObjectsQueue.poll()) != null)
    {
      WorkViewModel.delete(reference._nativeHandle);
      phantomReferences.remove(reference);
    }
  }
}
Bruno Bieri
  • 9,724
  • 11
  • 63
  • 92
  • 1
    I believe immediately that this approach does not scale. But I don’t understand your second issue that you “*haven't found a way to create a base class … which would take any objects and would handle the native deletion properly*”. You have a working solution consisting of two classes. What problem should that hypothetical base class solve and how? – Holger Dec 14 '17 at 12:11
  • @Holger thanks for your reply. Please explain me the scale issue? That is what I try to solve with a hypothetical base class. I've many many such objects and for each of those I've to create a second phantom class. With a base class I would like to solve that I wouldn't need to create an extra class or make it that simple that I only need to define the type. – Bruno Bieri Dec 14 '17 at 12:30
  • 1
    Wait—you create a new phantom class for each object you create? Or what do you mean with “each of those”? – Holger Dec 14 '17 at 13:24
  • Exactly that is what I do at the moment (third paragraph in my question above). So that's why I ask this question if there is a better way to do this. – Bruno Bieri Dec 14 '17 at 13:55
  • 1
    It looks like your mixing up objects and classes. I don’t see why you should create a new *class* for each *object*. I suppose, you need a new *class* for each distinct *class* having a different `delete` method, right? And you need another new phantom *object* for each *object* needing a cleanup. When I read “does not scale” first, I thought, you were talking about the poor performance of such a design, which is partly related the the objects you have to create, however, it seems you are primarily talking about the code complexity, which is related to the classes. The latter might be solvable – Holger Dec 14 '17 at 16:43
  • @Holger Yes, exactly I mixed it up. Do you have any suggestions how the "latter might be solved"? – Bruno Bieri Dec 15 '17 at 06:32

1 Answers1

8

You may have a look at Java 9’s Cleaner API, which addresses a similar task, cleanup built around a PhantomReference, and implement a similar thing, adapted it to your needs. Since you don’t need to support multiple cleaners, you can stay with a static registration method. I recommend to keep the abstraction of the reference, i.e. the Cleanable interface, to ensure that no inherited reference method can be invoked, especially as clear() and clean() are easy to confuse:

public class Cleaner {
    public interface Cleanable {
        void clean();
    }
    public static Cleanable register(Object o, Runnable r) {
        CleanerReference c = new CleanerReference(
                Objects.requireNonNull(o), Objects.requireNonNull(r));
        phantomReferences.add(c);
        return c;
    }
    private static final Set<CleanerReference> phantomReferences
                                             = ConcurrentHashMap.newKeySet();
    private static final ReferenceQueue<Object> garbageCollectedObjectsQueue
                                              = new ReferenceQueue<>();

    static final class CleanerReference extends PhantomReference<Object>
                                        implements Cleanable {
        private final Runnable cleaningAction;

        CleanerReference(Object referent, Runnable action) {
            super(referent, garbageCollectedObjectsQueue);
            cleaningAction = action;
        }
        public void clean() {
            if(phantomReferences.remove(this)) {
                super.clear();
                cleaningAction.run();
            }
        }
    }
    public static void deleteOrphanedNativePeerObjects() {
        CleanerReference reference;
        while((reference=(CleanerReference)garbageCollectedObjectsQueue.poll()) != null) {
            reference.clean();
        }
    }
}

This uses Java 8 features; if ConcurrentHashMap.newKeySet() is not available, you may use Collections.newSetFromMap(new ConcurrentHashMap<CleanerReference,Boolean>()) instead.

It kept the deleteOrphanedNativePeerObjects() to trigger cleanup explicitly, but it is thread safe, so it would be no problem to create a daemon background thread to clean items as soon as they are enqueued, like in the original.

Expressing the action as Runnable allows to use this for arbitrary resources, and getting the Cleanable back allows to support explicit cleanup without relying on the garbage collector while still having the safety net for those objects that haven’t been closed.

public class WorkViewModel extends BaseObservable implements AutoCloseable
{
    private long _nativeHandle;
    Cleaner.Cleanable cleanable;

    public WorkViewModel(Database database, int workId)
    {
      _nativeHandle = create(database.getNativeHandle(), workId);
      cleanable = createCleanable(this, _nativeHandle);
    }
    private static Cleaner.Cleanable createCleanable(Object o, long _nativeHandle) {
        return Cleaner.register(o, () -> delete(_nativeHandle));
    }

    @Override
    public void close() {
        cleanable.clean();
    }

    private static native long create(long databaseHandle, int workId);
    static native void delete(long nativeHandle);

    @Bindable
    public native int getWorkId();
    public native void setWorkId(int workId);

}

By implementing AutoCloseable, it can be used with the try-with-resources construct, but also invoking close() manually is possible, if there is not a simple block scope. The advantage of closing it manually, is not only that the underlying resource gets closed much earlier, also the phantom object is removed from the Set and will never be enqueued, making the entire life cycle more efficient, especially when you create and use a lot objects in short terms. But if close() is not called, the cleaner gets enqueued by the garbage collector eventually.

For classes supporting to be closed manually, keeping a flag would be useful, to detect and reject attempts to use it after being closed.

If lambda expressions are not available for your target, you can implement Runnable via inner class; it’s still simpler than creating another subclass of the phantom reference. Care must be taken not to capture the this instance, that’s why the creation has been moved into a static method in the example above. Without a this in scope, it can’t be captured by accident. The method also declares the referent as Object, to enforce the usage of the parameter values instead of instance fields.

Holger
  • 285,553
  • 42
  • 434
  • 765
  • It works like a charm. Thank you very much. I've replaced the `Runnable` by an own class, since `Runnable` very often refers to use of threading and I didn't want to introduce any possible confusion about this. – Bruno Bieri Jan 19 '18 at 06:44
  • I had to use the `Collections.newSetFromMap(new ConcurrentHashMap())` call because the target version for the Android App is API Level 15. I ran into the following problem https://stackoverflow.com/a/25705596/1306012 – Bruno Bieri Feb 23 '18 at 11:56
  • 1
    Well, `Collections.newSetFromMap(…)` was what I also suggested in my answer for pre-Java 8 environments. Mind the difference between `newKeySet()` and `keySet()`: the former is a special `ConcurrentHashMap` factory method which only exists in Java 8 or newer, the latter is a general `Map` method that returns a set view to the keys of a map and doesn’t support `add` operations, hence, it wouldn’t be appropriate anyway. – Holger Feb 23 '18 at 12:14