[kepler-dev] change proposal for ptolemy.vergil.tree.EntityTreeModel class.

[Jianwu Wang]

  Hi Christopher,

     I did more explicit cleanup in Kepler code and found the current 
leak is gone even without the changes for 
ptolemy.vergil.tree.EntityTreeModel class. Another issue is that I found 
another error in my changes:  '_listenerList.remove(new 
WeakReference(listener));' will not really work. I attached a new 
version for this class.

     So there are two options for it now. We can either remove all my 
changes, or use the attached version. I also saw many similar listeners 
in Ptolemy, such as _valueListeners in 
ptolemy.kernel.util.ConfigurableAttribute and 
ptolemy.data.expr.Variable, which do not use weak reference. It might be 
good to keep them the same.

     Sorry for the mess up. I should dig more in Kepler before changing 
Ptolemy code.

On 9/24/10 10:39 AM, Christopher Brooks wrote:
> Hi Jianwu,
> Thanks, I folded this into the devel trunk and the 8.0 branch.
>
> _Christopher
>
> On 9/24/10 9:54 AM, Jianwu Wang wrote:
>> Hi Christopher,
>>
>> Since WeakReference.get() could return null, we'd better check it 
>> after using this function. So I changed this class a little big more. 
>> Please find new class from the attachment and check it in if you
>> think it is proper. Thanks.
>>
>> On 9/20/10 6:14 PM, Christopher Brooks wrote:
>>> Hi Jianwu,
>>>
>>> Your change looks ok to me. The diff between what you sent and what
>>> is in the tree is below:
>>>
>>> bash-3.2$ diff ~/Downloads/EntityTreeModelNew.java EntityTreeModel.java
>>> 30d29
>>> < import java.lang.ref.WeakReference;
>>> 64c63
>>> < public class EntityTreeModelNew implements TreeModel {
>>> ---
>>> > public class EntityTreeModel implements TreeModel {
>>> 70c69
>>> < public EntityTreeModelNew(NamedObj root) {
>>> ---
>>> > public EntityTreeModel(NamedObj root) {
>>> 82c81
>>> < _listenerList.add(new WeakReference(listener));
>>> ---
>>> > _listenerList.add(listener);
>>> 176c175
>>> < _listenerList.remove(new WeakReference(listener));
>>> ---
>>> > _listenerList.remove(listener);
>>> 188c187
>>> < TreeModelListener listener = (TreeModelListener) 
>>> ((WeakReference)listeners.next()).get();
>>> ---
>>> > TreeModelListener listener = (TreeModelListener) listeners.next();
>>> 263c262
>>> < private List _listenerList = new LinkedList<WeakReference>();
>>> ---
>>> > private List _listenerList = new LinkedList();
>>>
>>>
>>> I folded in the change and added a comment:
>>>
>>> /** Add a listener to this model.
>>> * @param listener The listener to add.
>>> * @see #removeTreeModelListener(TreeModelListener)
>>> */
>>> public void addTreeModelListener(TreeModelListener listener) {
>>> // In http://bugzilla.ecoinformatics.org/show_bug.cgi?id=4801,
>>> // Jianwu Wang found that the _listenerList attribute in
>>> // ptolemy.vergil.tree.EntityTreeModel was leaking memory.
>>> // This attribute keeps holding some references, so that
>>> // instances of EntityTreeModel and its sub-classes can not be
>>> // GCed when a window is open and closed.
>>> //
>>> // When it and its sub-classes are used in Kepler,
>>> // addTreeModelListener() and removeTreeModelListener()
>>> // functions of EntityTreeModel class are called implicitly.
>>> // Jianwu did not find a good way to clean up after a window
>>> // is closed. Changing the _listenerList attribute using
>>> // WeakReference fixe the leak.
>>> _listenerList.add(new WeakReference(listener));
>>> }
>>>
>>> I updated the 8.0 release branch, thanks for the patch!
>>>
>>> Derik, I think you would want this in the 2.1 release. I'm still 
>>> working on deterimining
>>> if ptolemy-8.0.0 is what should be shipped with Kepler 2.1.
>>>
>>> _Christopher
>>>
>>>
>>> On 9/20/10 4:27 PM, Jianwu Wang wrote:
>>>> As I mentioned at 
>>>> http://bugzilla.ecoinformatics.org/show_bug.cgi?id=4801, I found 
>>>> one class might need to be changed for memory leak. It's the 
>>>> _listenerList attribute in
>>>> ptolemy.vergil.tree.EntityTreeModel. One memory leak I'm tracing 
>>>> down is that this attribute keeps holding some references, so that 
>>>> instances of EntityTreeModel and its sub-classes can not be GCed
>>>> when a window is open and closed.
>>>>
>>>> When it and its sub-classes are used in Kepler, 
>>>> addTreeModelListener() and removeTreeModelListener() functions of 
>>>> EntityTreeModel class are called implicitly. I haven't find good 
>>>> ways to clean
>>>> up after a window is closed.
>>>>
>>>> If changing the _listenerList attribute using WeakReference, it 
>>>> will be able to fix the leak.
>>>
>>
>>
>


-- 

Best wishes

Sincerely yours

Jianwu Wang
jianwu at sdsc.edu

Assistant Project Scientist
Scientific Workflow Automation Technologies (SWAT) Laboratory
San Diego Supercomputer Center
University of California, San Diego
San Diego, U.S.A.

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: EntityTreeModel.java
URL: <http://mercury.nceas.ucsb.edu/kepler/pipermail/kepler-dev/attachments/20100924/0bfac44c/attachment-0001.pl>