Hi,
I’m afraid I found a memory leak in IndexedContainer.
Property value change listeners registered via IndexedContainerProperty.addListener(ValueChangeListener) are not dereferenced when calling IndexedContainer.removeAllItems().
A possible fix may be the attached one.
12551.txt (436 Bytes)
Please create a ticket of this at
dev.vaadin.com
.
filed a ticket:
#9786
I’m wondering what’s the contract of Property.ValueChangeNotifier, Item.PropertySetChangeNotifier and Container.PropertySetChangeNotifier regarding the removal of items and properties. Can I expect all listeners to be removed automatically, e.g. by removeAllItems, removeItem or removeContainerProperty? It seems that some containers (e.g. AbstractBeanContainer) clear listeners on removeAllItems.
My opinion is that Vaadin should remove all listeners properly when an item is removed. That would only make sense… But then again, although I work for Vaadin, I don’t design the actual framework. Ultimately these decisions are up to the dev team. I’ll flag this thread for them, let’s see if they have anything to add 
My personal opinions on this:
In general, items can outlive the container they were created for, and could still send value change events after the container ceases to exist. However, IndexedContainer is a special case as the properties use data directly from the container, their listeners are managed on the container level and each property has a reference to the container.
If the same item id (and property id) were added again to the container, the old property instance would probably refer to it in the current implementation, but depending on such behavior is not safe.
Therefore, I believe ticket
#9786
is valid (although some might disagree): IndexedContainer, unlike some other containers, should clear the single property value change listeners when the item is removed. IndexedContainer.addContainerProperty() and removeContainerProperty() need to be looked at - not sure if the issue also concerns them.
The item property value change listeners e.g. in AbstractBeanContainer are used for a different purpose (internal to the container, re-filtering the container on property value change) and are not the same item property change listeners that are exposed outside the container. These internal listeners are removed when the item is removed from the container. Each property of each item manages its own listeners, the container does not even know about them nor does the item or property know about the container, and an item and its properties can live independently of the lifetime of the container.
Note that if the container is discarded without removing its items and a reference to an item is kept in memory, an item could still hold onto it. I’m afraid this cannot be handled easily on the framework level - transient listeners cannot be used because of cluster solutions, GAE etc.
As a general rule, each class that adds listeners for an item should eventually clean them, but not modify other listeners unless it logically invalidates them.