Search code examples
c#readonly-collection

ReadOnlyCollection property underlying Collection is modified while iterating


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);
            }
        }
    }

Solution

  • 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.