Pros and Cons of Listeners as WeakReferences

Question

What are the pros and cons of keeping listeners as WeakReferences?

The big 'Pro' of course is that:

Adding a listener as a WeakReference means the listener doesn't need to bother 'removing' itself.

For those worried about the listener having the only reference to the object, why can't there be 2 methods, addListener() and addWeakRefListener()?

Those who don't care about removal can use the latter.

Solution 1

First of all, using WeakReference in listeners lists will give your object different semantic, then using hard references. In hard-reference case addListener(...) means "notify supplied object about specific event(s) until I stop it explicitly with removeListener(..)", in weak-reference case it means "notify supplied object about specific event(s) until this object will not be used by anybody else (or explicitly stop with removeListener)". Notice, it is perfectly legal in many situations to have object, listening for some events, and having no other references keeping it from GC. Logger can be an example.

As you can see, using WeakReference not just solve one problem ("I should keep in mind to not forget to remove added listener somewhere"), but also rise another -- "I should keep in mind that my listener can stop listen at any moment when there is no reference to it anymore". You not solve problem, you just trade one problem for another. Look, in any way you've forced to clearly define, design and trace livespan of you listener -- one way or another.

So, personally, I agree with mention what use WeakReference in listeners lists is more like a hack than a solution. It's pattern worth to know about, sometimes it can help you -- to make legacy code work well, for example. But it is not pattern of choice :)

P.S. Also it should be noted what WeakReference introduce additional level of indirection, which, in some cases with extremely high event rates, can reduce performance.

Solution 2

This is not a complete answer, but the very strength you cite can also be its principal weakness. Consider what would happen if action listeners were implemented weakly:

button.addActionListener(new ActionListener() {
    // blah
});

That action listener is going to get garbage collected at any moment! It's not uncommon that the only reference to an anonymous class is the event to which you are adding it.

Solution 3

I have seen tons of code where listeners were not unregistered properly. This means they were still called unnecessarily to perform unnecessary tasks.

If only one class is relying on a listener, then it is easy to clean, but what happens when 25 classes rely on it? It becomes much trickier to unregister them properly. The fact is, your code can start with one object referencing your listener and end up in a future version with 25 objects referencing that same listener.

Not using WeakReference is equivalent to taking a big risk of consuming unnecessary memory and CPU. It is more complicated, trickier and requires more work with hard references in the complex code.

WeakReferences are full of pros, because they are cleaned up automatically. The only con is that you must not forget to keep a hard reference elsewhere in your code. Typically, that would in objects relying on this listener.

I hate code creating anonymous class instances of listeners (as mentioned by Kirk Woll), because once registered, you can't unregister these listeners anymore. You don't have a reference to them. It is really bad coding IMHO.

You can also null a reference to a listener when you don't need it anymore. You don't need to worry about it anymore.

Solution 4

To be honest I don't really buy that idea and exactly what you expect to do with a addWeakListener. Maybe it is just me, but it appear to be a wrong good idea. At first it is seducing but the problems it might implies are not negligible.

With weakReference you are not sure that the listener will no longer be called when the listener itself is no longer referenced. The garbage collector can free up menmory a few ms later or never. This mean that it might continue to consume CPU and make strange this like throwing exception because the listener shall not be called.

An example with swing would be to try to do things you can only do if your UI component is actually attached to an active window. This could throw an exception, and affect the notifier making it to crash and preventing valid listeners to be notofied.

Second problem as already stated is anonymous listener, they could be freed too soon never notified at all or only a few times.

What you are trying to achieve is dangerous as you cannot control anymore when you stop receiving notifications. They may last for ever or stop too soon.

Solution 5

There are really no pros. A weakrefrence is usually used for "optional" data, such as a cache where you don't want to prevent garbage collection. You don't want your listener garbage collected, you want it to keep listening.

Update:

Ok, I think I might have figured out what you are getting at. If you are adding short-lived listeners to long-lived objects there may be benefit in using a weakReference. So for example, if you were adding PropertyChangeListeners to your domain objects to update the state of the GUI that is constantly being recreated, the domain objects are going to hold on to the GUIs, which could build up. Think of a big popup dialog that is constantly being recreated, with a listener reference back to an Employee object via a PropertyChangeListener. Correct me if I'm wrong, but I don't think the whole PropertyChangeListener pattern is very popular anymore.

On the other hand, if you are talking about listeners between GUI elements or having domain objects listening to GUI elements, you won't be buying anything, since when the GUI goes away, so will the listeners.

Here are a couple interesting reads:

http://www.javalobby.org/java/forums/t19468.html

How to resolve swing listener memory leaks?

View more solutions

Comments

[skinnypinny]

why cant there be 2 methods, addListener() and addWeakRefListener()? those who dont care about removal can use the latter

[Kirk Woll]

@pdeva, yes, that would work. It's a bit dangerous though that people have to remember which method to call. I don't think this pattern is necessarily something you should always avoid, but it better have a good reason -- and IMO that reason cannot simply be "prevent mistakes by forgetful programmers."

[skinnypinny]

"prevent mistakes by forgetful programmers." Isnt that why GC was invented? :) Also it keeps your code simpler since you dont have to remember to remove the listener when disposing an object.

[Kirk Woll]

@pdeva, I do not mean it's a bad principle to uphold. But I believe the medicine is at least as bad as the symptom -- if they forget to use the medicine properly (add the listener using the wrong method) they'll be in even worse shape than the normal solution.

[alphazero]

You can get the array of listeners for a given component. Anonymous classes can extend a specialization. Give that specialized listener class a uid property and you are all set for removing anon listeners selectively.

[Bringer128]

Another problem of using the WeakReference - "I should keep in mind that any state changes caused by my Listener may still happen after it has fallen out of scope". Because of this a WeakReference Listener will need to be designed with the knowledge that its existence is non-deterministic.

[Bringer128]

The anonymous listener that you dislike should only be registered if it should never be unregistered. If you save it to a variable/field/collection for later de-registration then would you consider that acceptable?

[Manuel Leuenberger]

While I agree with you that anonymous listeners are a double-edged sword, the same is true for me for using weak references: To null a reference to the listener to remove it, is somewhat anti-semantic. You'd need to comment this line, that this means that the listener is unregistered any time in the future. In my opinion, the programmer is responsible for registering listeners, so he's responsible to remove them too.

[BegemoT]

Yes, you definitely right. But it seems from my expirience what where is rather many cases when it is not an issue -- what is why the question is arise :) And you can see examples of using WeakReference for listeners even in jdk code, as far as I recall :)

[Jérôme Verstrynge]

@maenu I totally agree that it is the programmer's responsibility to register and unregister listeners. No discussion about that. What I am trying to argue is that it is much simpler and less risky to use weak references...

[Bringer128]

Do you have any references for the WeakReference being used as a listener in the jdk? I'd be interested to see where it is used.

[BegemoT]

@Bringer128 Well, it seems I was wrong. I've seen many uses of WR-based listeners around swing, but looking sources I can't find any examples inside. JDK uses more interesting technic -- listeners list itself uses ordinary reference, but concrete listener serves as adapter, holding only WeakRef to actual object reciving events -- and it deregister itself as target GC-ed. Interesting :)

[supercat]

How about: An object raises an event when something happens. You would like to have an object with a field or method which reports how many times that has happened. If no reference to that object exists outside the event which increments that field, the object should cease to exist. If many objects which monitor a long-lived object are constructed and abandoned, then unless the event is subscribed weakly, the objects will continue to accumulate without bound.

[vir us]

Can't say about JDK, but Android SDK uses one - SharedPreference is one of the examples. It uses WaekHashMap to store registered listeners. grepcode.com/file/repository.grepcode.com/java/ext/… May be naming methods which exploits weakrefs like addWeakListener(listener) may be a good practice as user of this class will have an idea what is going on inside to avoid missing notifies

[glebm]

What if garbage collection happens after the call to on() but before keepFor()? Presumably keepFor() calls weakReference.get() that may return null already at that point?

[Christian]

@glebm, yes, you are right, that is a problem. very unlikely but that makes it even worse. Keeping it forever by default, until another method is called would fix that problem. Or just make the reference from the returned object to the listener not weak. So it won't get garbage collected as long as the returned object is in use.

[Jan Chrbolka]

This is a 3 year old question, And your "answer" does not seem to be one.

[Teto]

I did apologize explicitly for resurrecting a 3 year old post. But when I read this thread it was new to me and I am sure others will stumble across this for the first time too if they Google "weak listener". I think those people will benefit from my contribution to the discussion as a whole. In particular, I cited a specific example where weak listeners were used in a modern Java API (which had been brought up). I also addressed ways of mitigating the "risks" of using weak listeners by suggesting the use of "self removal requires a log entry" policy.

[user515655]

The proper type of reference for 'optional' data such as caching is a SoftReference, not a WeakReference. WeakReference is generally used for preventing reference leaks, explicitly getting higher finalization priority over SoftReferences such that they don't interfere with the storage of cached items during GC sweeps.

[user515655]

"What you are trying to achieve is dangerous as you cannot control anymore when you stop receiving notifications." - THIS. WeakReferences allow a specially-written listener to be automatically removed, but it's still only when the GC darn well feels like it, which could be never and frequently is.

[Dmitry Ryadnenko]

Not using weak referenced callbacks leads to the world where each object has to have "destructor". And this defeats whole point of having GC.

[Don Hatch]

Here's another con of relying on weak references-- let's say I have 1000 listeners (views) onto an evolving simulation (model) and the views can come and go very dynamically. And let's say some user action (e.g. interactive window resize) can cause all the views to go away and be recreated, say 100 times. I then have 100,000 lapsed listeners... if the observerd holds them as WRs, they will eventually be cleaned up by GC... but until that time, they will all be listening and doing possibly expensive work, causing significant (temporary) slowdown.

[Don Hatch]

I wish people would omit the "or stop too soon" part of this argument-- I think that part is a distraction, since that's just a programming error which requires care to avoid but it's not a fundamental problem, and when you mention it it can make people tend to dismiss your whole argument when visually scanning over your answer. The "may last indefinitely long" part is the real problem, which you rightly point out.

[Nicolas Bousquet]

The stop too soon appear in different cases but is as problematic. If it is just a listener that you'd hope to stay for the life of the application or the same life as the component creating the event to listen to and your listener is garbage collected just after its creation this is a big problem too.