[BBB-ISSUES] [JIRA] Commented: (BETTERBEANSBINDING-57) Memory leak in PropertyHelper class

["brad103 (JIRA)" <jira-no-reply@kenai.com>]

    [ 
http://kenai.com/jira/browse/BETTERBEANSBINDING-57?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=47727#action_47727
 ] 

brad103 commented on BETTERBEANSBINDING-57:
-------------------------------------------

mikhail, you are my new friend!

I can confirm this bug with the following unit test:

{code}
public class ELPropertyTest {
        @Test
        public void testDetach() throws Exception {

                BoundObject sourceObject = new BoundObject();
                BoundObject targetObject = new BoundObject();

                ELProperty<Object, Object> prop = 
ELProperty.create("${watchedValue}");
                AutoBinding<Object, Object, Object, Object> binding = 
Bindings.createAutoBinding(UpdateStrategy.READ_WRITE,
                                sourceObject, prop, targetObject, 
BeanProperty.create("watchedValue"));

                binding.bind();

                // Our source object now knows it's being watched
                assertEquals(1, 
sourceObject.changeSupport.getPropertyChangeListeners().length);
                // The listeners map in PropertyHelper has now been created
                assertEquals(1, 
prop.getPropertyStateListeners(sourceObject).length);

                binding.unbind();

                // Source object no longer watched
                assertEquals(0, 
sourceObject.changeSupport.getPropertyChangeListeners().length);
                // The listeners map in PropertyHelper now has no values for 
our source
                // object
                assertEquals(0, 
prop.getPropertyStateListeners(sourceObject).length);

                // Have to use reflection to check the keys of the listeners 
map
                Field listenersField = 
prop.getClass().getSuperclass().getDeclaredField("listeners");
                listenersField.setAccessible(true);
                Object listeners = listenersField.get(prop);
                IdentityHashMap<?, List<PropertyStateListener>> map = 
(IdentityHashMap<?, List<PropertyStateListener>>) listeners;

                // Show the listeners map's keys
                System.out.println(map.keySet());
                // As per ticket #57 by mikhail.maximov, the map still has a 
reference
                // to our source object
                assertEquals(0, map.size());
        }

        public class BoundObject {
                public PropertyChangeSupport changeSupport = new 
PropertyChangeSupport(this);

                private String watchedValue = "default";

                public String getWatchedValue() {
                        return watchedValue;
                }

                public void setWatchedValue(String watchedValue) {
                        String oldWatchedValue = this.watchedValue;
                        this.watchedValue = watchedValue;
                        changeSupport.firePropertyChange("watchedValue", 
oldWatchedValue, watchedValue);
                }

                public void addPropertyChangeListener(PropertyChangeListener 
listener) {
                        changeSupport.addPropertyChangeListener(listener);
                }

                public void 
removePropertyChangeListener(PropertyChangeListener listener) {
                        changeSupport.removePropertyChangeListener(listener);
                }
        }
}{code}

It may seem not to be a big problem here, as 'binding' has no references once 
detached so will be gc'd anyway. However, when binding is something more 
persistent, such as a ColumnBinding on a JTable, then its listeners keySet 
just keeps growing. I've witnessed this in my code and I plan to add another 
test here to demonstrate.

The suggested fix does solve the problem. Good job mikhail!

As I have code in production that is suffering memory leaks causing crashes, 
I will be deploying my own BBB.jar to fix it prior to an official release. 
Fabrizio, it would be good to get a release with this in ASAP, thanks.

> Memory leak in PropertyHelper class
> -----------------------------------
>
>                 Key: BETTERBEANSBINDING-57
>                 URL: http://kenai.com/jira/browse/BETTERBEANSBINDING-57
>             Project: BetterBeansBinding
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.3.0
>            Reporter: mikhail.maximov
>            Assignee: fabriziogiudici
>            Priority: Critical
>   Original Estimate: 1 hour
>  Remaining Estimate: 1 hour
>
> There is memory leak in the PropertyHelper.removePropertyStateListener 
> method. When the PropertyHelper is created with ignoresSource=false (e.g. 
> any ELProperty), the removePropertyStateListener  removes given listener 
> for the source object. However, if this was the last listener for given 
> source, the mapping for the source is not removed and it remains in the 
> listener map.
> Suggested fix is to replace the following snippet at the method end
> {code:java}
> if (wasListening && (listeners.size() == 0)) {
>       listeningStopped(ignoresSource ? null : source);
> }
> {code}
> with 
> {code:java}
> if (wasListening && listeners.size() == 0) {
>             if (!ignoresSource) {
>                 IdentityHashMap<S, List<PropertyStateListener>> map = 
> (IdentityHashMap<S, List<PropertyStateListener>>)this.listeners;
>                 map.remove(source);
>             }
>             listeningStopped(ignoresSource ? null : source);
>         } 
> {code}

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: 
http://kenai.com/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira