ReadOnlyCollection property underlying Collection is modified while iterating

Question

I have the following property:

private static Collection<Assembly> _loadedAssemblies = new Collection<Assembly>();
    internal static ReadOnlyCollection<Assembly> LoadedAssemblies
    {
        get { return new ReadOnlyCollection<Assembly>(_loadedAssemblies); }
    }

In another class I loop through LoadedAssemblies

foreach (Assembly assembly in ResourceLoader.LoadedAssemblies)

While looping through the assemblies the underlying collection (_loadedAssemblies) changes sometimes which gives a System.InvalidOperationException. What is the preferred way to make LoadedAssemblies safe? I cannot reproduce the problem so I can't just try it.

Is it okay to do?

internal static ReadOnlyCollection<Assembly> LoadedAssemblies
    {
        get { return new ReadOnlyCollection<Assembly>(_loadedAssemblies.ToList()); }
    }

Edit

    public static void Initialize()
    {

        foreach (Assembly assembly in AppDomain.CurrentDomain.GetAssemblies())
        {
            AddAssembly(assembly);
        }
        AppDomain currentDomain = AppDomain.CurrentDomain;
        currentDomain.AssemblyLoad += OnAssemblyLoad;
    }

    private static void OnAssemblyLoad(object sender, AssemblyLoadEventArgs args)
    {
        AddAssembly(args.LoadedAssembly);
    }

    private static void AddAssembly(Assembly assembly)
    {
        AssemblyName assemblyName = new AssemblyName(assembly.FullName);
        string moduleName = assemblyName.Name;

        if (!_doesNotEndWith.Exists(x => moduleName.EndsWith(x, StringComparison.OrdinalIgnoreCase)) &&
            _startsWith.Exists(x => moduleName.StartsWith(x, StringComparison.OrdinalIgnoreCase)))
        {
            if (!_loadedAssemblies.Contains(assembly))
            {
                _loadedAssemblies.Add(assembly);
            }
        }
    }

Why it changes sometimes? Thats causing the exception. You cannot modify a collection in a foreach. — Tim Schmelter, Aug 15 '17 at 07:45
@TimSchmelter it looks like the underlying collection is modified which causes an exception when iterating,how to fix that? — Sybren, Aug 15 '17 at 07:49
don't modify it while enumerating, show the code that modifies it — Tim Schmelter, Aug 15 '17 at 07:49
thats one option but i guess i have to do changes with a lot of impact, any other options? — Sybren, Aug 15 '17 at 07:52
it just the foreach in the question (2nd snippet) in a method in another class, it does not modify the loadedassemblies collection — Sybren, Aug 15 '17 at 08:28
How important is it that you have the latest version of the underlying collection while looping? ie if the underlying collection does change to add a new collection what do you actually want to do? Carry on with the old collection lacking the new elements or take some action to restart your loop or find some way to work out what elements are new and deal with just them? — Chris, Aug 15 '17 at 08:45
@Chris I want to carry on with the old collection lacking the new elements — Sybren, Aug 15 '17 at 08:48

score 3 · Answer 1 · answered Aug 15 '17 at 07:58

ReadOnlyCollection<T> is just a wrapper around the original collection which prevents direct modifications. But it doesn't prevent the underlying collection to be modified if you expose it somewhere.

Since ReadOnlyCollection<T>.GetEnumerator is just returning an enumerator for the underlying collection it has the same rules. You cannot modify it while enumerating it, that means you are not allowed to add or remove items.

There is no easy fix without seeing your code, prevent that multiple threads access the underlying collection at the same time, f.e. with a lock. If you modify it in the foreach it might be simpler to fix, but we need to see that code then.

score 3 · Accepted Answer · answered Aug 15 '17 at 08:54

The suggestion you have in your question will work. That is:

internal static ReadOnlyCollection<Assembly> LoadedAssemblies
{
    get { return new ReadOnlyCollection<Assembly>(_loadedAssemblies.ToList()); }
}

The reason is that your problems come from _loadedAssemblies being changed while you are enumerating over it. This of course happens because ReadOnlyCollection is just a wrapper around _loadedAssemblies which uses the enumerator of the base collection.

If you do _loadedAssemblies.ToList() then this will create a new list that is a copy of the original _loadedAssemblies. It will have all the same elements at the time of creation but will never be updated again (since you don't even have a reference to the new collection you are unable to modify it). This means that when _loadedAssemblies is updated your new list inside the ReadOnlyCollection is blissfully unaware of that change and so your enumeration will continue without problem to the end.

ReadOnlyCollection property underlying Collection is modified while iterating

2 Answers2