[Biojava-dev] memory leak

[Rainer Pudimat]

Hello Mark,

I have tried your suggested change in ChangeSuppot.removeChangeListener, 
but without positive effect on my heap.

The problem is that the condition
if( (listeners[i].get() == cl) && (types[i] == ct) )
is never fullfilled so your change is never executed.

But anyway, it seems the right place to look for.
Inspired by the method next to removeChangeListener
(reapGarbageListener) I tried to test what Objects
to which References listeners[i] points to, are actually
null:

if( ((listeners[i].get() == cl) && (types[i] == ct)) || 
listeners[i].get() == null )

This is true in many cases.

On the other hand, the method reapGarbageListener was never
executed, independent of the number of
System.gc() calls.

Rainer.

mark.schreiber at group.novartis.com schrieb:
> 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
> 
> 
>