Why does ControlCollection NOT throw InvalidOperationException?

Question

Following this question Foreach loop for disposing controls skipping iterations it bugged me that iteration was allowed over a changing collection:

For example, the following:

List<Control> items = new List<Control>
{
    new TextBox {Text = "A", Top = 10},
    new TextBox {Text = "B", Top = 20},
    new TextBox {Text = "C", Top = 30},
    new TextBox {Text = "D", Top = 40},
};

foreach (var item in items)
{
    items.Remove(item);
}

throws

InvalidOperationException: Collection was modified; enumeration operation may not execute.

However in a .Net Form you can do:

this.Controls.Add(new TextBox {Text = "A", Top = 10});
this.Controls.Add(new TextBox {Text = "B", Top = 30});
this.Controls.Add(new TextBox {Text = "C", Top = 50});
this.Controls.Add(new TextBox {Text = "D", Top = 70});

foreach (Control control in this.Controls)
{
    control.Dispose();
}

which skips elements because the the iterator runs over a changing collection, without throwing an exception

bug? aren't iterators required to throw InvalidOperationException if the underlaying collection changes?

So my question is Why does iteration over a changing ControlCollection NOT throw InvalidOperationException?

Addendum:

The documentation for IEnumerator says:

The enumerator does not have exclusive access to the collection; therefore, enumerating through a collection is intrinsically not a thread-safe procedure. Even when a collection is synchronized, other threads can still modify the collection, which causes the enumerator to throw an exception.

Jarid

does this.Controls.Count chages after the foreach is complete? I think the controllCollection itself is not changing. – Libre

@AmitKumarGhosh Yes it does, since the ControllsCollection rearranges the index of the remaining Controls after each Control.ControlCollection.Remove() call – Something

Not all collections implement that feature. – Grams

For anyone else looking, this is where a Control removes itself from the parent collection when being disposed. – Phage

@Serv But you are not calling Control.ControlCollection.Remove() ? – Avulsion

@ChrisWohlert It's called/removed on your behalf during the dispose - see the link in my above comment. – Phage

@JamesThorpe I see. :) – Avulsion

ctl.parent = null; - this is interesting. But how is the parent affected? – Libre

@AmitKumarGhosh That's looping through the child controls of the one already being disposed. The child's parent for all intents and purposes is already gone, so makes sense to set it to null before calling the child dispose – Phage

I get that, So the parent isn't at all affected? – Libre

@AmitKumarGhosh It's code within the parent that's setting the child's parent to null. IE it's disassociating itself from its own child as it's already half way through being disposed. Given that one of the things that happens is that the child calls Remove on it's parent's child collection during disposal, you probably want to break that link first. – Phage

Answer 1

The answer to this can be found in the Reference Source for ControlCollectionEnumerator

private class ControlCollectionEnumerator : IEnumerator {
    private ControlCollection controls; 
    private int current;
    private int originalCount;

    public ControlCollectionEnumerator(ControlCollection controls) {
        this.controls = controls;
        this.originalCount = controls.Count;
        current = -1;
    }

    public bool MoveNext() {
        // VSWhidbey 448276
        // We have to use Controls.Count here because someone could have deleted 
        // an item from the array. 
        //
        // this can happen if someone does:
        //     foreach (Control c in Controls) { c.Dispose(); }
        // 
        // We also dont want to iterate past the original size of the collection
        //
        // this can happen if someone does
        //     foreach (Control c in Controls) { c.Controls.Add(new Label()); }

        if (current < controls.Count - 1 && current < originalCount - 1) {
            current++;
            return true;
        }
        else {
            return false;
        }
    }

    public void Reset() {
        current = -1;
    }

    public object Current {
        get {
            if (current == -1) {
                return null;
            }
            else {
                return controls[current];
            }
        }
    }
}

Pay particular attention to the comments in MoveNext() which explicitly address this.

IMO this is a misguided "fix" because it masks an obvious error by introducing a subtle one (elements are silently skipped, as noted by the OP).

Deliquesce

+1 So that explains how it avoids it. But should it not be throwing InvalidOperationException there? Documentation would imply any IEnumerator should do so. If that implication is correct, then its a bug right? But given the comments in the source-code, its explicitly avoiding it. Very odd. – Jarid

@MeirionHughes I agree - it looks like they tried to fix a perceived issue, and by doing so made things worse (if you consider it worse to have a silent bug instead of a loud bug, which I do) – Deliquesce

Until someone from ms-team explains WHY they explicitly didn't want to throw InvalidOperationException, this will be the accepted-answer. :) – Jarid

A design choice as they often assert. – Libre

Answer 2

This same issue of an exception not being thrown was raised in the comments on foreach control c# skipping controls. That question uses similar code except the child Control is explicitly removed from Controls before calling Dispose()...

foreach (Control cntrl in Controls)
{
    if (cntrl.GetType() == typeof(Button))
    {
        Controls.Remove(cntrl);
        cntrl.Dispose();
    }
}

I was able to find an explanation for this behavior through documentation alone. Basically, that modifying any collection while enumerating always causes an exception to be thrown is an incorrect assumption; such a modification causes undefined behavior, and it's up to the specific collection class how to handle that scenario, if at all.

According to the remarks for the IEnumerable.GetEnumerator() and IEnumerable<>.GetEnumerator() methods...

If changes are made to the collection, such as adding, modifying, or deleting elements, the behavior of the enumerator is undefined.

Classes such as Dictionary<>, List<>, and Queue<> are documented to throw an InvalidOperationException when modified during enumeration...

An enumerator remains valid as long as the collection remains unchanged. If changes are made to the collection, such as adding, modifying, or deleting elements, the enumerator is irrecoverably invalidated and the next call to MoveNext or IEnumerator.Reset throws an InvalidOperationException.

It's worth calling attention to the fact that it's each class I mentioned above, not the interfaces they all implement, that specifies the behavior of explicit failure via an InvalidOperationException. Thus, it's up to each class whether it fails with an exception or not.

Older collection classes such as ArrayList and Hashtable specifically define the behavior in this scenario as undefined beyond the enumerator being invalidated...

An enumerator remains valid as long as the collection remains unchanged. If changes are made to the collection, such as adding, modifying, or deleting elements, the enumerator is irrecoverably invalidated and its behavior is undefined.

...although in testing I found that enumerators for both classes do, in fact, throw an InvalidOperationException after being invalidated.

Unlike the above classes, the Control.ControlCollection class neither defines nor comments on such behavior, hence the above code failing in "merely" a subtle, unpredictable way with no exception explicitly indicating failure; it never said it would explicitly fail.

So, in general, modifying a collection during enumeration is guaranteed to (likely) fail, but not guaranteed to throw an exception.

Breastpin