[Biojava-dev] memory leak

[mark.schreiber at group.novartis.com]

The problem seems to be in ChangeSupport.removeChangeListener()

  /**
   * Remove a listener that was interested in a specific types of changes.
   *
   * @param cl  a ChangeListener to remove
   * @param ct  the ChangeType that it was interested in
   */
  public void removeChangeListener(ChangeListener cl, ChangeType ct) {
    synchronized(this) {
      for(int i = 0; i < listenerCount; i++) {
        if( (listeners[i].get() == cl) && (types[i] == ct) ) {
          listenerCount--;
          System.arraycopy(listeners, i+1, listeners, i, (listenerCount - 
i));
          System.arraycopy(types, i+1, types, i, (listenerCount - i));
          return;
        }
      }
    }

I think that somewhere there needs to be a nullification of the value at 
listeners[i]. This is a classic memory leak problem outlined in Item 5 of 
Joshua Bloch's Effective Java. I think that the loop should be:

 
  public void removeChangeListener(ChangeListener cl, ChangeType ct) {
    synchronized(this) {
      for(int i = 0; i < listenerCount; i++) {
        if( (listeners[i].get() == cl) && (types[i] == ct) ) {

          //prevent memory leak
          listeners[i] = null;

          listenerCount--;
          System.arraycopy(listeners, i+1, listeners, i, (listenerCount - 
i));
          System.arraycopy(types, i+1, types, i, (listenerCount - i));
          return;
        }
      }
    }

Does this look correct to everyone? I'm still a little concerned about 
that HashMap though as I don't see it here. It also doesn't address the 
question of what are all these ChangeListeners needed for (but then I'm no 
expert on the Changeable API).

- Mark





Mark Schreiber/GP/Novartis at PH
Sent by: biojava-dev-bounces at portal.open-bio.org
01/27/2005 01:25 PM

 
        To:     Michael Heuer <heuermh at acm.org>
        cc:     rpudimat at informatik.uni-jena.de, biojava-dev at biojava.org, Michael Heuer 
<heuermh at shell3.shore.net>, (bcc: Mark Schreiber/GP/Novartis)
        Subject:        Re: [Biojava-dev] memory leak


>    synchronized(this) {
>      growIfNecessary();
>      types[listenerCount] = ct;
>      listeners[listenerCount] = new WeakReference(cl);
>      listenerCount++;
>    }


This may be the source of the problem. The thing the WeakReference points 
to is released when memory runs low but a reference to the WeakReference 
is being maintained (possibly in the listeners[]).

- Mark
_______________________________________________
biojava-dev mailing list
biojava-dev at biojava.org
http://biojava.org/mailman/listinfo/biojava-dev