0

I'm reading Effective Java by Joshua Bloch. In Item 7: Eliminate Obsolete Object References of chapter 2 he states:

A third common source of memory leaks is listeners and other callbacks. If you implement an API where clients register callbacks but don’t deregister them explicitly, they will accumulate unless you take some action. One way to ensure that callbacks are garbage collected promptly is to store only weak references to them, for instance, by storing them only as keys in a WeakHashMap.

I understand the concept of a WeakHashMap. I implemented one for a mock cache:

/** Example courtesy of Baeldung */
public class WeakHashMapDemo {
    public static void main(String[] args) {

        /** Ideally, we want a Map implementation that allows GC to automatically delete unused objects.
         * When a key of a big image object is not in use in our application in any place,
         * that entry will be deleted from memory.
         */
        WeakHashMap<UniqueImageName, BigImage> wMap = new WeakHashMap<>();
        HashMap<UniqueImageName, BigImage> map = new HashMap<>();
        BigImage bigImage = new BigImage("image_id");
        UniqueImageName imageName = new UniqueImageName("name_of_big_image");

        /** We are putting a BigImage object as a value and an imageName object reference as a key. */
        wMap.put(imageName, bigImage);
        System.out.println("Map does not contain imageName -> " + wMap.containsKey(imageName));

        wMap.put(imageName, bigImage);

        /** The key is set to null. */
        imageName = null;
        /** Forcing JVM to GC. */
        System.gc();
        System.out.println("Weak Map is empty -> " + wMap);
    }

    private static class UniqueImageName {
        public UniqueImageName(String name_of_big_image) {
        }
    }

    private static class BigImage {
        public BigImage(String image_id) {
        }
    }
}

I also read this excellent article on weak references. However, I still don't understand using these concepts in the context of callbacks. How can I ensure memory leaks don't happen with a given API, using a WeakHashMap, if clients don't explicitly deregister them?

I appreciate any help you can provide.

UPDATE:

It seems a few people need more clarity on the question. When I mean by "How can I ensure memory leaks don't happen with a given API, using a WeakHashMap, if clients don't explicitly deregister them?" is how this can be implemented with code as I only seem to have a conceptual understanding at the moment. So I'd really appreciate it if someone could show me a code snippet on how this would be implemented on a mock API. Thanks again!

Rahul
  • 543
  • 1
  • 7
  • 24
  • @Holger Thanks for your comment. I have updated the question with more clarity about what I'm looking for. Does that make sense? Cheers! – Rahul Nov 09 '22 at 23:10

2 Answers2

3

The idea is to use a collection with weak semantics but since there is no such built-in collection implementation, store the listeners as keys of a WeakHashMap instead; the value is irrelevant. You can use Collections.newSetFromMap(…) to get a Set view to a Map which does exactly that for you. E.g.

public class WeakListenerExample {
    interface ExampleListener {
        void notify(String event);
    }

    final Set<ExampleListener> listeners= Collections.newSetFromMap(new WeakHashMap<>());

    public void addExampleListener(ExampleListener l) {
        listeners.add(l);
    }

    public void removeExampleListener(ExampleListener l) {
        listeners.remove(l);
    }

    protected void notifyAllListeners(String event) {
        for(ExampleListener l: listeners) l.notify(event);
    }
}

That said, I strongly discourage from using weak listeners. Using weak references does not “ensure that callbacks are garbage collected promptly”. The garbage collector still is non-deterministic and may not run for a long time or never at all.

This is no problem for caches where memory consumption is your only concern. If the garbage collector doesn’t run, it’s usually a sign that there is no memory pressure, hence, it doesn’t matter when the obsolete cached object stays in memory.

However, a listener encapsulates an action and you don’t want the action to be performed when the listener became obsolete. Unnecessarily performing an action can be a performance problem, even if all side effects are limited to an otherwise-unused object. Even worse, during the event notification, the listener is strongly reachable. It’s possible that even when the garbage collector runs, it overlaps with an unintended execution of a listener method, especially with concurrent garbage collectors.

In short, using a weak reference for a listener does not guaranty that it automatically becomes unregistered, not to speak of “promptly”.


This, however, is not the worst problem. Weak listeners are just trading one potential problem “you have to remember to unregister” for another “you have to remember to keep a strong reference”.

With a typical example of using listeners for binding code, e.g.

eventSource.addActionListener(event -> otherComponent.setFoo(event.getNewFoo()));

the event source is the only object having a reference to the listener. So if it is using weak references, the listener is eligible to garbage collection right after the registration. To prevents this, another reference to the listener, with ordinary, “strong”, semantic must be kept somewhere.

However, as explained above, garbage collection is non-deterministic. It’s unpredictable when this listener gets collected, hence, the problem may not materialize during testing but waiting for the moment with the worst outcome in production environments.

So, while forgetting to unregister an ordinary listener has a deterministic outcome (the listener stays registered and will be kept informed of new events), hence, is easy to analyze, forgetting to keep an additional strong reference to a weak listener is the other category. It may not occur in your testing/debug environment or exhibit an entirely different timing.

And fixing such issues brings you back to square one. Whoever registers a listener has to consider how long it ought to be registered and, in case of weak listeners, take measures to ensure that the object’s lifetime matches that intended time. And, controlling an object’s lifetime, how hard can it be?

Holger
  • 285,553
  • 42
  • 434
  • 765
1

How can I ensure memory leaks don't happen with a given API, using a WeakHashMap, if clients don't explicitly deregister them?

A client of the API has to keep a strong reference to the listener, that's how it stays alive.

The API only holds a weak reference to the listener, to that it does not prevent the client from being garbage collected. When the client is garbage collected, the listener is also garbage collected.

Like this:

class ApiUser {
    Listener listener;
    void registerListener() {
        listener = new Listener() {
            @Override
            public void event() {
                // TODO
            }
        };
        Api.registerListener(listener);
    }
}

class Api {
    static WeakHashMap<Listener, Object> listeners = new WeakHashMap<>();
    static void registerListener(Listener listener) {
         listeners.put(listener, null);
    } 

    static void fireListeners() {
        for (Listener listener : listeners.keySet()) {
            if (listener != null) {
                listener.event();
            }
        }
    }
}

interface Listener {
    void event();
}
Ingo Kegel
  • 46,523
  • 10
  • 71
  • 102
  • Thanks for that, Ingo. But I need a bit more clarification on what you meant by the client keeping a strong reference but API holding a weak reference to the listener, if you don't mind. Could you clarify with an example, perhaps? – Rahul Nov 09 '22 at 23:06
  • 1
    I've added some code – Ingo Kegel Nov 10 '22 at 09:12