maui icon indicating copy to clipboard operation
maui copied to clipboard

Fix BindableLayout BindingContext inheritance

Open PureWeen opened this issue 2 years ago • 0 comments

Description of Change

Usually the BindingContext is automatically inherited via Element.SetParent using SetChildInheritedBindingContext. When an element is removed from a Layout, the Element.SetParent takes care of clearing BindingContext automatically.

		void SetParent(Element value) {
			// ..
			object context = value?.BindingContext;
			if (value != null)
			{
				value.SetChildInheritedBindingContext(this, context);
			}
			else
			{
				SetInheritedBindingContext(this, null);
			}

We can see that setting the BindingContext to null is an explicit behavior when the parent (value) is set to null (a.k.a. child removed).

BindableLayout sets the BindingContext manually on each created child, so it is its responsibility to clear the BindingContext once the item is removed.

Not clearing the BindingContext might cause unwanted side-effects and leaks if the child view attached to some of its events (i.e. PropertyChanged).

// inside child view class
private MyViewModel _current;
protected override void OnBindingContextChanged() {
    if (_current != null) { 
        // I see no other way to unsubscribe to that event
        // Even if the event is backed by a WeakEventManager there's a risk
        // that an event is invoke before GC finishes cleaning up resources
        _current.MyEvent -= MyHandler;
        _current = null;
    }


    if (BindingContext != null) {
        // Subscribing to an event is totally legit here
        _current = BindingContext;
        _current.MyEvent += MyHandler;
    }
}

On top of that, the view created by EmptyViewTemplate should not have the BindingContext set manually, because it matches the parent one: we should rely on the automatic inheritance.

Issues Fixed

Issue #10904 does not represent a leak itself, but in a more complex scenario where a child attaches to some events on the BindingContext it would create issues.

PureWeen avatar Nov 30 '23 20:11 PureWeen