ObservableCollection.Clear leaving phantom elements

Question

I have a set of converters that are used dynamically position squares on a canvas. The squares are stored in an observable collection. CanvasPositionScaleConverter converts a value within a range to a value between 0 and 1. If the value it is converting is outside the specified range, it will throw an Argument exception.

My issue is when I clear my squares collection, whilst the screen becomes empty, "phantom" elements seem to be left behind which the converters continue to act on. Therefore, when the canvas is resized, I still get exceptions being thrown, even if the squares collection has been cleared.

Why are the converters still running on these elements that have been deleted through Squares.Clear()?

Note

Using Squares = new ObservableCollection() instead of Squares.Clear() still causes this issue.

Update

This issue does not happen if the Sqaures collection is never added too and Resize is pressed. It only happens if elements have been deleted from the collection (and obviously if haven't been removed from the collection but are outside the valid range).

The exception details are

Exception thrown: 'System.ArgumentException' in LateExceptionMcve.dll
An unhandled exception of type 'System.ArgumentException' occurred in LateExceptionMcve.dll
Value cannot be greater than max

MCVE

MainWindow.xaml

MainWindow.xaml.cs

using System;
using System.Collections.ObjectModel;
using System.ComponentModel;
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Windows;
using System.Windows.Data;

namespace LateExceptionMcve
{
    public partial class MainWindow : INotifyPropertyChanged
    {
        public MainWindow()
        {
            InitializeComponent();
            DataContext = this;
            Min = 0;
            Max = 10;
            Size = Max - Min;
            Squares = new ObservableCollection
            {
                new Square
                {
                    X = 8,
                    Y = 7,
                },
            };
        }

        public event PropertyChangedEventHandler PropertyChanged;

        private void OnPropertyChanged([CallerMemberName] string propertyName = null)
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }

        private void Resize(object sender, RoutedEventArgs e)
        {
            Min = 0;
            Max = 5;
            Size = Max - Min;
        }

        private void Clear(object sender, RoutedEventArgs e)
        {
            Squares.Clear();
        }

        private ObservableCollection _squares;

        public ObservableCollection Squares
        {
            get => _squares;
            set
            {
                _squares = value;
                OnPropertyChanged();
            }
        }

        private double _size;

        public double Size
        {
            get => _size;
            set
            {
                _size = value;
                OnPropertyChanged();
            }
        }

        private double _min;

        public double Min
        {
            get => _min;
            set
            {
                _min = value;
                OnPropertyChanged();
            }
        }

        private double _max;

        public double Max
        {
            get => _max;
            set
            {
                _max = value;
                OnPropertyChanged();
            }
        }
    }

    public sealed class Square : INotifyPropertyChanged
    {
        public event PropertyChangedEventHandler PropertyChanged;

        private void OnPropertyChanged([CallerMemberName] string propertyName = null)
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }

        private double _x;

        public double X
        {
            get => _x;
            set
            {
                _x = value;
                OnPropertyChanged();
            }
        }

        private double _y;

        public double Y
        {
            get => _y;
            set
            {
                _y = value;
                OnPropertyChanged();
            }
        }
    }

    public sealed class CanvasScaleConverter : IValueConverter
    {
        private const double Scale = 100;

        public object Convert(
            object value,
            Type targetType,
            object parameter,
            CultureInfo culture)
        {
            if (value is double valueToScale)
            {
                return valueToScale * Scale;
            }

            return value;
        }

        public object ConvertBack(
            object value,
            Type targetType,
            object parameter,
            CultureInfo culture)
        {
            throw new NotSupportedException();
        }
    }

    public sealed class CanvasPositionScaleConverter : IMultiValueConverter
    {
        public object Convert(
            object[] values,
            Type targetType,
            object parameter,
            CultureInfo culture)
        {
            if (values.Length == 4 &&
                values[0] is double value &&
                values[1] is double min &&
                values[2] is double max &&
                values[3] is double canvasWidthHeight)
            {
                return canvasWidthHeight * RangeToNormalizedValue(min, max, value);
            }

            return values;
        }

        private static double RangeToNormalizedValue(
            double min,
            double max,
            double value)
        {
            if (min > max)
            {
                throw new ArgumentException("Min cannot be less than max");
            }

            if (value < min)
            {
                throw new ArgumentException("Value cannot be less than min");
            }

            if (value > max)
            {
                throw new ArgumentException("Value cannot be greater than max");
            }

            return (value - min) / (max - min);
        }

        public object[] ConvertBack(
            object value,
            Type[] targetTypes,
            object parameter,
            CultureInfo culture)
        {
            throw new NotSupportedException();
        }
    }
}

Edit

I have filled a bug report on the WPF GitHub repo, so hopefully that may help find what is going on here.

user_4601149 · Answer

Information from miloush on the GitHub issue.

The cause of this issue is that even though the Squares collection has been cleared and the objects are no longer visible on the screen, the objects have not been garbage collected.

It is possible to force the garbage collection to happen by modifying the Clear method to something like

private void Clear(object sender, RoutedEventArgs e)
{
    Squares.Clear();
    Dispatcher.BeginInvoke(new Action(GC.Collect), DispatcherPriority.ContextIdle);
}

And this will (eventually) clear the objects away. However, it is not immediate. Even if just GC.Collect() after Squares.Clear had been called it is not immediate.

This issue could occur at many times in WPF, including in common actions such as virtualisation. Therefore it would be better to update the converter so it can handle invalid inputs gracefully.

public sealed class CanvasPositionScaleConverter : IMultiValueConverter
{
    public object Convert(
        object[] values,
        Type targetType,
        object parameter,
        CultureInfo culture)
    {
        if (values.Length != 4 ||
            !(values[0] is double value) ||
            !(values[1] is double min) ||
            !(values[2] is double max) ||
            !(values[3] is double canvasWidthHeight))
        {
            return null;
        }

        try
        {
            return canvasWidthHeight * RangeToNormalizedValue(min, max, value);
        }
        catch (ArgumentException e) when (e.Message == ValueLessThanMin)
        {
            return 0;
        }
        catch (ArgumentException e) when (e.Message == ValueGreaterThanMax)
        {
            return canvasWidthHeight;
        }
    }

    // This is in a utility class normally
    private const string ValueLessThanMin = "Value cannot be less than min";
    private const string ValueGreaterThanMax = "Value cannot be greater than max";

    private static double RangeToNormalizedValue(
        double min,
        double max,
        double value)
    {
        if (min > max)
        {
            throw new ArgumentException("Min cannot be less than max");
        }

        if (value < min)
        {
            throw new ArgumentException(ValueLessThanMin);
        }

        if (value > max)
        {
            throw new ArgumentException(ValueGreaterThanMax);
        }

        return (value - min) / (max - min);
    }

    public object[] ConvertBack(
        object value,
        Type[] targetTypes,
        object parameter,
        CultureInfo culture)
    {
        throw new NotSupportedException();
    }
}

ObservableCollection.Clear leaving phantom elements

1 Answers