DataDecoratorBase and PropertyStatus cause memory leaks

jureleskovec posted on Wednesday, September 09, 2009

Hi Rocky

After spending many hours tracing a performance issue in our WPF app I ran on the fact that the two classes have the following problems in connection with attaching and detaching event handlers:

1) DataDecoratorBase attaches to the data object's 'PropertyChanged' event 2 times, because the DataContext_Changed is in my case triggered before the Panel_Loaded call.

2) None of the objects (of mentioned types) unsubscribe from the 'PropertyChanged' event when the host form closes. The implication is grave: After you open a form for editing a business object it won't get collected until the BO gets unused too. This means that opening forms using the same BO (acting as a data source) will cause memory leaks.


I made a quick fix to these two classes and it works:
1) Unhook from the old and hook to the new data object event.
2) Always hook to the 'current' data object event if not hooked yet.
3) Always unhook from the 'current' object event if hooked yet

The 2) and the 3) are important because loaded and unloaded events can be called many times in a control's life - every time you attach and detach it from the visual tree (also by setting its Visibility to Collapsed). And the 3) is what is currently missing which cause the unsubscription after the form closes.

We're still using V3.6.2 but I also checked the source code of the Csla 3.7 and found no difference.


-- Jure

Fintanv replied on Thursday, September 10, 2009

Would it be possible for you to provide your code changes?

I ran into an issue with the PropertyStatus losing its connection to the business object on a DataProvider 'Refresh' operation.  I had to manually re-hook the Source to the PropertyStatus controls, and am wondering if your changes would fix this problem.

Thanks

jureleskovec replied on Friday, September 11, 2009

I doubt that your issue has anything to do with the mine. From my understanding of how the 'PropertyStatus' control works I can assert that the control only subscribes itself to the 'PropertyChanged' event of the source object and listens to the changes. In my case the problem is that the control doesn't get deteached form the event until the source object dies. In your case I'm not sure what the problem is because I'm not sure what you mean with 'losing its connection' and I have almost no experience with the data provider control.

Anyway, here are the changes made to the ProertyStatus.cs:
1) Add the private field:
private bool _isAttachedToSource;

2) Add or modify methods
public PropertyStatus()
    {
      DefaultStyleKey = typeof(PropertyStatus);
      BrokenRules = new ObservableCollection<BrokenRule>();

      Loaded += (o, e) => { UpdateState(); };
      this.Loaded += new RoutedEventHandler(Control_Loaded);
      this.Unloaded += new RoutedEventHandler(Control_Unloaded);
    }

private void Control_Loaded(object sender, RoutedEventArgs e)
    {
        if (!_isHookedToPropertyChanged) AttachSource(Source);
    }

    private void Control_Unloaded(object sender, RoutedEventArgs e)
    {
        if (_isHookedToPropertyChanged) DetachSource(Source);
    }

private void AttachSource(object source)
        {
            // Ignore the call if already attached.
            if (_isAttachedToSource) return;

            INotifyBusy busy = source as INotifyBusy;
            if (busy != null)
                busy.BusyChanged += new BusyChangedEventHandler(source_BusyChanged);

            INotifyPropertyChanged changed = source as INotifyPropertyChanged;
            if (changed != null)
            {
                changed.PropertyChanged += new PropertyChangedEventHandler(source_PropertyChanged);
            }

            _isAttachedToSource = true;
        }

        private void DetachSource(object source)
        {
            // Ignore the call if not attached.
            if (!_isAttachedToSource) return;

            INotifyBusy busy = source as INotifyBusy;
            if (busy != null)
                busy.BusyChanged -= new BusyChangedEventHandler(source_BusyChanged);

            INotifyPropertyChanged changed = source as INotifyPropertyChanged;
            if (changed != null)
            {
                changed.PropertyChanged -= new PropertyChangedEventHandler(source_PropertyChanged);
            }

            _isAttachedToSource = false;
        }


-- Jure

jureleskovec replied on Tuesday, September 22, 2009

No respond?