Posts Tagged ‘JDateChooser’

Java Swing memory leak: JDialog, JDateChooser, and the evil of Singleton’s

Saturday, August 16th, 2008

I recently solved a mysterious memory leak puzzle in a java swing application that I’m working on. The source of the memory leak was proving elusive, even with people on the team using JProbe in order to find the source of the leak.

At the root of the problem, the Dialog was too complex and with too much logic to easily pinpoint the problem. Circularly referential object graphs were everywhere. I had to rip out most of the dialog in large chunks until I reduced it down to a simple JDialog, and then considered each possible factor.

I was a little tripped up along the way. Apparently, calling Runtime.getRuntime().gc() doesn’t necessarily collect memory, but if push push the JVM to the point of an OutOfMemoryError, it’ll do everything in it’s power to get all the available memory back. Pushing it the edge was a sure way to get an honest answer.

Normally, when Java memory leaks happen in Swing, the culprit is a listener of some kind. It’s easy to register a listener and forget about it, while it continues to point to other objects with a strong reference. It’s further complicated by the fact that it’s not always obvious when a reference is created, like in the case of defining an anonymous inner class.

In my case I discovered that the culprit of the memory leak was a JDateChooser object that we are using. That object is defined as part of the JCalendar api. Specifically, inside the dialog, a PropertyChangeListener was anonymously created and registered to the JDateChooser, creating a circular reference from the JDateChooser and the dialog.

So, what was JDateChooser doing? Sure enough, in the constructor, it was registering a listener with a Swing singleton called MenuSelectionManager. That singleton never dies, and does not releasing the reference so that the garbage collector can do it’s magic.

The code in the JDateChooser constructor:

  1. // Corrects a problem that occured when the JMonthChooser’s combobox is
  2. // displayed, and a click outside the popup does not close it.
  3.  
  4. // The following idea was originally provided by forum user
  5. // podiatanapraia:
  6. changeListener = new ChangeListener() {
  7.  public void stateChanged(ChangeEvent e) {
  8.     // do stuff that creates a reference back to the JDateChooser
  9.  }
  10. };
  11. MenuSelectionManager.defaultManager().addChangeListener(changeListener);

I love the comment. It’s just too bad that the forum user didn’t recommend constructor injection of that manager, which would have allowed me an elegant means of preventing the memory leak without modifying the library.

For instance, I could make a WeakReferenceMenuSelectionManager that stores the listeners using WeakReferences - thus allowing the garbage collector to reclaim the objects in the event that there are no other objects with a strong reference to the listener.

Unfortunately, when the maker of JDateChooser realized they were causing memory leaks, they came up with this solution:

  1. /**
  2. * Should only be invoked if the JDateChooser is not used anymore. Due to popup
  3. * handling it had to register a change listener to the default menu
  4. * selection manager which will be unregistered here. Use this method to
  5. * cleanup possible memory leaks.
  6. */
  7. public void cleanup() {
  8.   MenuSelectionManager.defaultManager().removeChangeListener(changeListener);
  9.   changeListener = null;
  10. }

This is better than nothing, but is still far inferior to a WeakReference approach. This is an excellent example of why we shouldn’t write code that reaches out and calls a Singleton. statically referencing an object does not allow me to reconfigure the object without changing the source code - something I would prefer not to do in a library. Luckily, the JCalendar api is covered under the LGPL, so at least changing the library was an option.

The better fix was this. First, I create a weak change listener as a proxy around the original change listener.

  1. public class WeakChangeListenerProxy implements ChangeListener {
  2.  
  3.     public WeakReference reference;
  4.  
  5.     public WeakChangeListenerProxy(ChangeListener listener) {
  6.         this.reference = new WeakReference(listener);
  7.     }
  8.  
  9.     public void stateChanged(ChangeEvent e) {
  10.         ChangeListener actualListener = (ChangeListener)reference.get();
  11.         if (actualListener != null) {
  12.             actualListener.stateChanged(e);
  13.         }
  14.     }
  15. }

Then, In JDateChooser, I changed the constructor like so:

  1. // The following idea was originally provided by forum user
  2. // podiatanapraia:
  3. ChangeListener changeListener2 = new ChangeListener() {
  4.     // Change listener body
  5. };
  6.      
  7. changeListener = new WeakChangeListenerProxy(changeListener2);
  8. MenuSelectionManager.defaultManager().addChangeListener(changeListener);

Finally, I wanted to clean up the remaining WeakReference object. Now that the JDateChooser will get garbage collected, I can do that by creating a finalize method.

  1. protected void finalize() throws Throwable {
  2.     super.finalize();
  3.     MenuSelectionManager.defaultManager().removeChangeListener(changeListener);
  4. }

I posted this solution to the JCalendar forum, hopefully, it’ll get added to the next release. The cleanup() method is not an obvious solution. As a user of a widget api (especially in Java), I wouldn’t expect that I need to free up any resources, and indeed, a lot of time and money was wasted with developers looking into this issue.

Oh well, at least I got to have some fun with Weak References.

Tags: , , ,
Posted in General Agile | 3 Comments »