1

I have a combo box that holds a list of values followed by a static "Add New" item. When I select that item, it loads an image and adds the image's file name to the list of values. When I do this, however, the WPF underlying code throws the "collection modified" exception.

XAML:

<StackPanel Orientation="Vertical">
    <ComboBox x:Name="selector">
        <ComboBoxItem IsEnabled="False" Content="---" />
        <ComboBoxItem FontStyle="Italic" Content="Add New" Selected="New_Selected" />
    </ComboBox>
</StackPanel>

Code:

public partial class MainWindow : Window
{
    List<string> files = new List<string>();

    public MainWindow()
    {
        InitializeComponent();
    }

    private void RepopulateResourceSelector()
    {
        // Remove all but the bottom 2 items
        while (selector.Items.Count > 2)
        {
            selector.Items.RemoveAt(0);
        }

        int index = 0;

        // Add all strings in the list to combo box
        foreach (var file in files)
        {
            selector.Items.Insert(index, file);
            index++;
        }
    }

    private void New_Selected(object sender, RoutedEventArgs e)
    {
        var dlg = new OpenFileDialog();
        dlg.Filter = "Image Files (.bmp, .jpg, .gif, .png, .tiff)|*.bmp;*.jpg;*.gif;*.png;*.tiff";

        if (dlg.ShowDialog(this) == true)
        {
            // Add selected file to the list
            string name = System.IO.Path.GetFileNameWithoutExtension(dlg.FileName);
            files.Add(name);

            RepopulateResourceSelector();
        }

        // Deselect `Add New` item
        selector.SelectedIndex = -1;
    }
}

Stack Trace:

System.InvalidOperationException occurred
    HResult=0x80131509
    Message=Collection was modified; enumeration operation may not execute.
Source=<Cannot evaluate the exception source>
StackTrace:
    at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
    at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
    at System.Collections.Generic.List`1.Enumerator.MoveNext()
    at System.Windows.Controls.Primitives.Selector.SelectionChanger.CreateDeltaSelectionChange(List`1 unselectedItems, List`1 selectedItems)
    at System.Windows.Controls.Primitives.Selector.SelectionChanger.End()
    at System.Windows.Controls.Primitives.Selector.SelectionChanger.SelectJustThisItem(ItemInfo info, Boolean assumeInItemsCollection)
    at System.Windows.Controls.ComboBox.NotifyComboBoxItemMouseUp(ComboBoxItem comboBoxItem)
    at System.Windows.Controls.ComboBoxItem.OnMouseLeftButtonUp(MouseButtonEventArgs e)
    at System.Windows.UIElement.OnMouseLeftButtonUpThunk(Object sender, MouseButtonEventArgs e)
    at System.Windows.Input.MouseButtonEventArgs.InvokeEventHandler(Delegate genericHandler, Object genericTarget)
    at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
    at System.Windows.RoutedEventHandlerInfo.InvokeHandler(Object target, RoutedEventArgs routedEventArgs)
    at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
    at System.Windows.UIElement.ReRaiseEventAs(DependencyObject sender, RoutedEventArgs args, RoutedEvent newEvent)
    at System.Windows.UIElement.OnMouseUpThunk(Object sender, MouseButtonEventArgs e)
    at System.Windows.Input.MouseButtonEventArgs.InvokeEventHandler(Delegate genericHandler, Object genericTarget)
    at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
    at System.Windows.RoutedEventHandlerInfo.InvokeHandler(Object target, RoutedEventArgs routedEventArgs)
    at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
    at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)
    at System.Windows.UIElement.RaiseTrustedEvent(RoutedEventArgs args)
    at System.Windows.UIElement.RaiseEvent(RoutedEventArgs args, Boolean trusted)
    at System.Windows.Input.InputManager.ProcessStagingArea()
    at System.Windows.Input.InputManager.ProcessInput(InputEventArgs input)
    at System.Windows.Input.InputProviderSite.ReportInput(InputReport inputReport)
    at System.Windows.Interop.HwndMouseInputProvider.ReportInput(IntPtr hwnd, InputMode mode, Int32 timestamp, RawMouseActions actions, Int32 x, Int32 y, Int32 wheel)
    at System.Windows.Interop.HwndMouseInputProvider.FilterMessage(IntPtr hwnd, WindowMessage msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
    at System.Windows.Interop.HwndSource.InputFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
    at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
    at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
    at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
    at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
    at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
    at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
    at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
    at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
    at System.Windows.Threading.Dispatcher.PushFrame(DispatcherFrame frame)
    at System.Windows.Application.RunDispatcher(Object ignore)
    at System.Windows.Application.RunInternal(Window window)
    at System.Windows.Application.Run(Window window)
    at System.Windows.Application.Run()
    at WpfApp1.App.Main()
Abion47
  • 22,211
  • 4
  • 65
  • 88
  • it's because you cannot remove from the `selector` at the same time you are going through the it – jamiedanq Jun 11 '17 at 20:09
  • @jamiedanq I'm not going through it. It's a `while` loop, not a `for`. – Abion47 Jun 11 '17 at 20:25
  • @jamiedanq And like I said in my question, the exception isn't happening anywhere in my code. It's happening somewhere in the WPF code _after_ my code completes. – Abion47 Jun 11 '17 at 20:25
  • 1
    The first six entries in the stack trace of your exception, what do they tell you? I don't know why you are trying to alter the selection if the selection is just changing; however, you might try modifying the selection after the Selected and SelectionChanged events have been concluded. For example, by scheduling the respective work on the event queue of the dispatcher (BeginInvoke or InvokeAsync) –  Jun 11 '17 at 21:57
  • Does the exception occur due to this line `RepopulateResourceSelector();` or this line `selector.SelectedIndex = -1;` (i.e. comment out one of them, then the other, to see which is causing it)? It is probably the former but I just want to check before giving advice. – mjwills Jun 11 '17 at 23:20
  • Have you consider binding the ComboBox to an ObservableCollection (via ItemsSource )? And then altering the entries by altering the ObservableCollection instead)? – mjwills Jun 11 '17 at 23:26
  • @elgonzo This is the solution. I got the error to not happen by wrapping the `RepopulateResourceSelector` call in a `Dispatcher.BeginInvoke`. If you want to post it as an answer I will mark it. – Abion47 Jun 12 '17 at 00:40
  • @mjwills Yes, I tried that, and it wasn't working at all - even when I prepopulated the collection with items, the combo box would always be empty. – Abion47 Jun 12 '17 at 00:42

3 Answers3

0

You have a synchronisation problem with RepopulateResourceSelector(), that exception tells you that the Collection(which is an Enumeration) that you are iterating through is being modified at the same time in the same thread, that is why wrapping it with Dispatcher.BeginInvoke() solved your problem (here is a detailed explanation on how it works) and if I'm right, it could also be solved adding a lock, something like this:

private Object filesLock = new Object();

private void RepopulateResourceSelector()
{
    // Remove all but the bottom 2 items
    while (selector.Items.Count > 2)
    {
        selector.Items.RemoveAt(0);
    }

    int index = 0;

    lock(filesLock){
        // Add all strings in the list to combo box
        foreach (var file in files)
        {
            selector.Items.Insert(index, file);
            index++;
        }
    }
}
Fabricio Monsalve
  • 109
  • 8
  • This is not a threading issue as WPF only operates on one thread by default. As such, I can't see why incorporating a lock would help. Like I said, the exception doesn't get thrown anywhere in my own code. It gets thrown after the event method returns and control reverts to the internal WPF code. – Abion47 Jun 12 '17 at 15:47
0

Instead of removing items from the ComboBox each time, you could use a CompositeCollection and an ObservableCollection to which you simply add the new items to.

Try this:

XAML:

<ComboBox x:Name="selector">
    <ComboBox.ItemsSource>
        <CompositeCollection>
            <CollectionContainer x:Name="cc" />
            <ComboBoxItem IsEnabled="False" Content="---" />
            <ComboBoxItem FontStyle="Italic" Content="Add New" Selected="New_Selected" />
        </CompositeCollection>
    </ComboBox.ItemsSource>
</ComboBox>

Code:

public partial class MainWindow: Window
{
    ObservableCollection<string> files = new ObservableCollection<string>();

    public MainWindow()
    {
        InitializeComponent();
        cc.Collection = files;
    }

    private void New_Selected(object sender, RoutedEventArgs e)
    {
        var dlg = new OpenFileDialog();
        dlg.Filter = "Image Files (.bmp, .jpg, .gif, .png, .tiff)|*.bmp;*.jpg;*.gif;*.png;*.tiff";

        if (dlg.ShowDialog(this) == true)
        {
            // Add selected file to the list
            string name = System.IO.Path.GetFileNameWithoutExtension(dlg.FileName);
            Dispatcher.BeginInvoke(new Action(() =>
            {
                files.Add(name);
                // Deselect `Add New` item
                selector.SelectedIndex = -1;
            }));
        }
    }
}
mm8
  • 163,881
  • 10
  • 57
  • 88
  • See my comment about having already tried this and it not working. – Abion47 Jun 12 '17 at 15:44
  • It works just fine for me so then you are doing something wrong. Did you really copy and paste my code? – mm8 Jun 12 '17 at 15:49
  • Also note the call to Dispatcher.BeginInvoke. – mm8 Jun 12 '17 at 16:04
  • When I tried the `CompositeCollection` approach, it wasn't throwing errors, but my combo box was not updating at all. I wasn't doing the `cc.Collection` assignment but instead calling `Add` on the `CollectionContainer` directly, so that might have been my issue. But regardless, it's working now, and I've moved onto other places of my program, so I'd rather not backtrack at this stage to fix what isn't currently broken. – Abion47 Jun 12 '17 at 16:48
  • And you should read the comments of the questions you are answering. I already said that I had a working solution 13 hours before you posted your answer. Yes, your answer works, but I already have a solution that I am comfortable enough with to not want to go back and replace it with something else when I have higher priorities elsewhere in my code to address at the moment. – Abion47 Jun 12 '17 at 17:20
  • Yep. And I told @elgonzo that if he posted his comment as an answer, I would mark it as the solution. – Abion47 Jun 12 '17 at 18:19
0

I had almost the exact same exception stack with a ListView (customised for touchscreen)

Setting IsSynchronizedWithCurrentItem="False" stopped it from crashing.

sergeantKK
  • 644
  • 7
  • 16