Skip to content

The "Collection was of a fixed size." exception when configuring options. #77725

Closed

Description

@martintmk

Description

Exception thrown from the collection when configuring and options. This bug happens when we configure the options using the Configure callback AFTER the Bind extension is called.

This happens when you are using non-concrete collection type in our options:

public class MyOptions
{
    public IList<string> MyList { get; set; } = new List<string>();
}

I suspect that the Bind extension materializes incorrect IList type that has a fixed size. and no more items can be added to it after that.

We use this pattern across our codebase, and it breaks our scenarios completely. It blocks us from migrating to .NET 7. Full repro source here

Reproduction Steps

Run the following code in .NET 7.0 RC2 console app:

var config = new ConfigurationBuilder()
    .AddInMemoryCollection(new Dictionary<string, string?>() {{ "MyList:0", "item1" }})
    .Build();

var services = new ServiceCollection();
services
    .AddOptions<MyOptions>()
    .Bind(config)
    .Configure(o => o.MyList.Add("xxx"));

var val = services.BuildServiceProvider().GetRequiredService<IOptions<MyOptions>>().Value;

Console.WriteLine("List size: {0}", val.MyList.Count);

public class MyOptions
{
    public IList<string> MyList { get; set; } = new List<string>();
}

Expected behavior

The code snippet above should work.

Actual behavior

When trying to add a new item to a list the list throws the following exception:

Unhandled exception. System.NotSupportedException: Collection was of a fixed size.

Regression?

No response

Known Workarounds

Change the collection to a concrete type. i.e. IList<string> -> List<string>

Configuration

Running on .NET 7 using the default console template.

Other information

No response

Activity

  1. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Nov 1, 2022
  2. geeknoid commented on Nov 1, 2022

    @geeknoid
    Member

    Very important thing to note. This is new behavior in .NET 7, this bug was not present in previous versions of .NET. This change in .NET behavior breaks a lot of our code and our customer's code.

  3. tarekgh commented on Nov 1, 2022

    @tarekgh
    Member

    We'll take a look soon at this one. Thanks for your report.

  4. tarekgh commented on Nov 1, 2022

    @tarekgh
    Member

    CC @SteveDunn as FYI.

  5. self-assigned this
    on Nov 1, 2022
  6. removed
    untriagedNew issue has not been triaged by the area owner
    on Nov 1, 2022
  7. SteveDunn commented on Nov 2, 2022

    @SteveDunn
    Contributor

    CC @SteveDunn as FYI.

    Hi, happy to fix, but unfortunately I'm not free until the weekend. Will this go into .NET 7?

  8. stephentoub commented on Nov 2, 2022

    @stephentoub
    Member

    What change regressed this, @tarekgh / @SteveDunn?

  9. tarekgh commented on Nov 2, 2022

    @tarekgh
    Member

    @stephentoub we are going to investigate as I mentioned in #77725 (comment)

  10. tarekgh commented on Nov 2, 2022

    @tarekgh
    Member
  11. stephentoub commented on Nov 2, 2022

    @stephentoub
    Member
  12. eerhardt commented on Nov 2, 2022

    @eerhardt
    Member

    Before #68133, the checks were fairly simple:

    // See if its a Dictionary
    Type collectionInterface = FindOpenGenericInterface(typeof(IDictionary<,>), type);
    if (collectionInterface != null)
    {
    BindDictionary(instance, collectionInterface, config, options);
    }
    else if (type.IsArray)
    {
    instance = BindArray((Array)instance, config, options);
    }
    else
    {
    // See if its an ICollection
    collectionInterface = FindOpenGenericInterface(typeof(ICollection<>), type);
    if (collectionInterface != null)
    {
    BindCollection(instance, collectionInterface, config, options);

    This would fall to BindCollection, which would just add to the existing object instance.

    Now, we are checking upfront if the property's type is compatible with an array, and if it is, copying the current instance's values + the config values into a new array:

    // for arrays, collections, and read-only list-like interfaces, we concatenate on to what is already there, if we can
    if (type.IsArray || IsArrayCompatibleInterface(type))
    {
    if (!bindingPoint.IsReadOnly)
    {
    bindingPoint.SetValue(BindArray(type, (IEnumerable?)bindingPoint.Value, config, options));

    This is incorrect for "mutable" collection types (like IList<T> and ICollection<T>). For those types, we should just be appending to the existing instance, if there is one.

  13. tarekgh commented on Nov 2, 2022

    @tarekgh
    Member

    @eerhardt I am working on a fix. We need to first create instance using List<T> if it is not created yet before binding. Anyway, let you know what I'll get.

  14. tarekgh commented on Nov 3, 2022

    @tarekgh
    Member

    I opened #77857 to fix this issue.

  15. geeknoid commented on Nov 4, 2022

    @geeknoid
    Member

    I presume the fix isn't going to make .NET 7's initial release?

  16. stephentoub commented on Nov 4, 2022

    @stephentoub
    Member

    Correct. It would be in a subsequent servicing release.

  17. added this to the 7.0.x milestone on Nov 4, 2022
  18. tarekgh commented on Nov 11, 2022

    @tarekgh
    Member

    This is fixed by the PR #78118. The fix will be in the next 7.0 service release.

  19. ghost locked as resolved and limited conversation to collaborators on Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions