September 26th, 2007


More Java Stupidity

Java's model-view system is fundamentally broken.

It assumes that the model object is only being held by its views. Sometimes this is a valid assumption, but other times, the model might be provided by a different, persistent part of the program -- perhaps a manager object that controls access to an external resource, or something similar.

The following example is based on my reading of the Java 1.6 source code. Consider the following, crudely-drawn ASCII art:

   Persistent ListModel
    |              /|\
    |               |
   \|/              |
ListDataListener    |
    |               |
    |               |
   \|/              |
    +---- JList ----+

The Persistent ListModel object is, as its name suggests, around for the life of the program. When the window gets garbage-collected, it drops its reference to the JList. At this point, what should happen is the JList should get garbage-collected along with its ListDataListener.

However, what actually happens is that since the persistent ListModel is still around, we still have a live reference to the ListDataListener (which is stored in the ListModel's listener list -- try saying that three times fast), which in turn has a reference (because it's an enclosed class) to the JList. So the ListDataListener and JList never get collected. We have a memory leak.

The memory leak by itself isn't that big a deal, unless your users (like my users) tend to leave their programs open for weeks on end. But what is a big deal is the slowdown caused by leaking listeners.

Since the ListDataListener never gets removed from the ListModel, it's still going to continue to receive model events. But we don't care about that JList anymore, so the time spent updating it is time wasted. Each time a new JList attaches itself to the ListModel, the ListModel will slow down just a little bit more.

Sun could have solved this in any number of ways. For example, they could have dispose()/finalize() methods on all Swing widgets that take care of it, or make EventListenerList use weak references (and require objects that care to keep strong references to their listeners).

But they did not -- the only way to correct the memory leak is to explicitly remove the model from its view before disposing of the window; that way the listener goes away. This is a rather messy solution, because it (a) requires developers to be aware of the subtle distinction between persistent and non-persistent models, and (b) implement their own cleanup code.

Worse, none of this is mentioned anywhere in the documentation.

-- Des
  • Current Mood
    annoyed annoyed
  • Tags