[Biojava-dev] memory leak

[Matthew Pocock]

Ah - do we have a parent-child inner/outer class hard/soft reference 
problem? If you want the sublist to be managed independantly of the 
parent, you should realy copy it. Is there a method in SymbolListTools 
or SequenceTools that does this?

Matthew

mark.schreiber at group.novartis.com wrote:

>Looking at this further.
>
>The parent SymbolList adds ChangeListeners to all it's children that are 
>created during subList() or subStr() operations. The children of these 
>subXXX operations are views on the parent. Any Edit operations performed 
>on them are notified to the parent and applied to the parent (who knew? 
>how cool!). So that answers the question about what they are for and also 
>why they are not cleaned up.
>
>Following your clue about the reapGarbageListeners() method your correct 
>that it is never called. The only place it can be called is from 
>ChangeSupport.firePreChangeEvent or ChangeSupport.firePostChangeEvent. 
>Because nothing we are doing in the loop is actually causing anything to 
>change there is no chance for reapListeners() to be called.
>
>To test this out I changed the loop to:
>
>    while(true){
>      SymbolList sl2 = sl.subList(1,12);
>      Edit e = new Edit(4, DNATools.getDNA(), DNATools.g());
>      sl2.edit(e);
>    }
>
>This forces a changeEvent and results in garbageCollection of the 
>WeakReferences and bingo, no more memory leak. 
>
>So, there needs to be some mechanism by which reapGarbageListeners() can 
>be called without the need for a changeEvent. I'm thinking that the 
>finalize() method of SimpleSymbolList might call it but this is not 
>currently possible as the reapListeners() method is protected and in a 
>different package. Maybe the finalize() method could throw a dummy 
>changeEvent as a bit of a hack and to signal to the world it is dying.
>
>Matthew or Thomas, Any ideas??
>
>- Mark
>
>
>
>
>
>
>Rainer Pudimat <rpudimat at informatik.uni-jena.de>
>01/27/2005 11:19 PM
>
> 
>        To:     biojava-dev at biojava.org, Mark Schreiber/GP/Novartis at PH
>        cc: 
>        Subject:        Re: [Biojava-dev] memory leak
>
>
>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
>>
>>
>>
>>    
>>
>
>
>
>
>
>  
>