how to create cleaner for map of string to file handles?

Question

I have a class

class Something {
    private Map<String, RandomAccessFile> map = new LinkedHashMap<>() {
        @Override
        protected boolean removeEldestEntry(...) { also closes the file if it returns true; }

        @Override
        public RandomAccessFile remove(Object filename) { closes the file too; }
    };
}

This is a map of filename to the file handle. The idea is to keep a few files open, so that member functions of the Something class can write to any of the open files.

Now I'd like to override finalize so that when the object goes out of scope, we close all files. But finalize is deprecated. So the idea is to call cleaner.register(map, runnable) where runnable gets all the RandomAccessFile in 'map' and closes each. I.e. something like cleaner.register(map, () -> map.values().forEach(close file)). But this makes runnable have a reference to the map, so the map can never become unreachable - class LinkedHashMap.LinkedValues is not a static class.

Any ideas how to do accomplish this?

Answer 1

You have to separate the object whose reachability should control the cleanup and the data needed for the cleanup.

In your case, the lifetime of the Something instance would determine whether the files are still in use, whereas the map instance can be used during the cleanup action, assuming that you keep the map private and never hand it out.

There are still some important points to care for. The documentation of Cleaner says:

The cleaning action could be a lambda but all too easily will capture the object reference, by referring to fields of the object being cleaned, preventing the object from becoming phantom reachable. Using a static nested class, as above, will avoid accidentally retaining the object reference.

When you use the lambda expression () -> map.values().forEach(close file)) and map is a field access of the current Something instance, you have implicitly captured this. But even if you fix the cleaner, to let it only reference the map instance, there’s the problem that the map itself is an anonymous subclass of LinkedHashMap and anonymous inner classes always have an implicit reference to their outer instance.

You can fix both by performing the initialization in a static context:

class Something {
    private Map<String, RandomAccessFile> map = initializeMapAndCleaner(this);

    static final Cleaner CLEANER = Cleaner.create();

    static Map<String, RandomAccessFile> initializeMapAndCleaner(Object instance) {
        LinkedHashMap<String, RandomAccessFile> map = new LinkedHashMap<>() {
            @Override
            protected boolean removeEldestEntry(Map.Entry<String, RandomAccessFile> eldest) {
                /*also closes the file if it returns true;*/
            }

            @Override
            public RandomAccessFile remove(Object filename) { /*closes the file too;*/ }
        };
        CLEANER.register(instance, () -> map.values().forEach(f -> {/*close file*/}));
        return map;
    }
}

By using a static method, there can’t be any implicit reference to this and the explicit reference provided in the instance parameter has been deliberately typed as Object, to make it harder to accidentally accessing a member in the map class or cleanup action.

But note that the LinkedHashMap does not guaranty that all removal operations go through that single overridden remove method. Further, replacement operations do not use remove. It may work for your internal use when that remove is the only operation (besides put) the Something class uses, but in this case, it would also be possible and cleaner to make file closing the caller’s duty.

---

That said, you should not implement the cleaner at all. If all the cleanup is doing, is to close the RandomAccessFile, it’s obsolete as there is already a cleaner closing the underlying resource when RandomAccessFile becomes unreachable without being closed. If relying on that cleaner feels hacky, you’re on the right track. But relying on Something’s cleaner is in no way better.

Generally, variable scopes and object reachability are only remotely connected. See Can java finalize an object when it is still in scope? or finalize() called on strongly reachable objects in Java 8 for a connected real life problem. But of course, the timing issues described in When is the finalize() method called in Java? apply to cleaners as well. The garbage collector may never run, e.g. when there’s enough memory, or it may run but not collect all unreachable objects, e.g. when it reclaimed enough memory for the application to proceed.

When you want to clean up at the end of a variable’s scope, the try-with-resources Statement is the way to go. You only have to implement AutoCloseable or a subtype of it, providing a close() method that will close all files.

class Something implements Closeable { // java.io.Closeable is suitable here
    …

    @Override
    public void close() throws IOException {
        for(RandomAccessFile f: map.values()) f.close();
        map.clear();
    }
}

Then you can easily say

try(Something sth = new Something(…)) {
    // use sth
}
// safely closed