diff --git a/src/System.Windows.Forms/System/Windows/Forms/Control.ControlCollection.cs b/src/System.Windows.Forms/System/Windows/Forms/Control.ControlCollection.cs index 6fa6774c1d0..45e4199beb4 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Control.ControlCollection.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Control.ControlCollection.cs @@ -72,6 +72,8 @@ public virtual void Add(Control? value) // Add the control InnerList.Add(value); + int _savedTabIndex = value._tabIndex; + if (value._tabIndex == -1) { // Find the next highest tab index @@ -101,6 +103,25 @@ public virtual void Add(Control? value) // you could end up with a control half reparented. value.AssignParent(Owner); } + catch + { + // AssignParent may throw for invalid parents, leaving the control partially added. + // Roll back the failed add to keep the collection and parent state consistent. + if (InnerList.Contains(value)) + { + InnerList.Remove(value); + } + + // AssignParent may have already changed the parent before throwing. + // Restore the previous parent to keep Parent and Controls collection in sync. + if (value._parent == Owner) + { + value._parent = oldParent; + } + + value._tabIndex = _savedTabIndex; + throw; + } finally { if (oldParent != value._parent && (Owner._state & States.Created) != 0) diff --git a/src/test/unit/System.Windows.Forms/System/Windows/Forms/Form.ControlCollectionTests.cs b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Form.ControlCollectionTests.cs index d7fd402ae18..a55b26d0f15 100644 --- a/src/test/unit/System.Windows.Forms/System/Windows/Forms/Form.ControlCollectionTests.cs +++ b/src/test/unit/System.Windows.Forms/System/Windows/Forms/Form.ControlCollectionTests.cs @@ -23,4 +23,38 @@ public void ControlCollection_Ctor_NullOwner_ThrowsArgumentNullException() { Assert.Throws("owner", () => new Form.ControlCollection(null)); } + + [WinFormsFact] + public void ControlCollection_Add_InvalidTabPageParent_DoesNotLeaveHalfAddedControl() + { + using Form form = new(); + using TabPage tabPage = new(); + + int oldCount = form.Controls.Count; + + Assert.ThrowsAny(() => form.Controls.Add(tabPage)); + + Assert.Equal(oldCount, form.Controls.Count); + Assert.False(form.Controls.Contains(tabPage)); + Assert.Null(tabPage.Parent); + } + + [WinFormsFact] + public void ControlCollection_Clear_AfterFailedTabPageAdd_DoesNotHang() + { + using Form form = new(); + using Button button = new() { Name = "button1" }; + using TabPage tabPage = new(); + + form.Controls.Add(button); + + Assert.ThrowsAny(() => form.Controls.Add(tabPage)); + + // If the failed add left the TabPage half-added, + // Clear() could hang. After the fix, it should complete normally. + form.Controls.Clear(); + + Assert.Empty(form.Controls); + Assert.Null(tabPage.Parent); + } }