From 8d3c5ecb1f99bfa4d83822858620f84a1ff10823 Mon Sep 17 00:00:00 2001 From: Misaki Date: Sat, 28 Mar 2026 22:37:59 +0900 Subject: [PATCH] fix(docking): address reentrancy and validation issues in DockContainer --- .../View/Controls/Docking/DockContainer.cs | 65 +++++++++++++------ .../View/Controls/Docking/DockGroup.cs | 18 +---- 2 files changed, 46 insertions(+), 37 deletions(-) diff --git a/src/Editor/Ghost.Editor/View/Controls/Docking/DockContainer.cs b/src/Editor/Ghost.Editor/View/Controls/Docking/DockContainer.cs index 8e160eb..4a63c8e 100644 --- a/src/Editor/Ghost.Editor/View/Controls/Docking/DockContainer.cs +++ b/src/Editor/Ghost.Editor/View/Controls/Docking/DockContainer.cs @@ -42,29 +42,15 @@ public abstract class DockContainer : DockModule /// The module to insert. public virtual void InsertChild(int index, DockModule module) { - ArgumentNullException.ThrowIfNull(module); + ValidateChild(module); if (index < 0 || index > _children.Count) throw new ArgumentOutOfRangeException(nameof(index)); - if (module == this) - throw new ArgumentException("Cannot add a container to itself.", nameof(module)); - - if (module is DockContainer container) - { - var current = Owner; - while (current != null) - { - if (current == container) - throw new ArgumentException("Cannot add a container that is an ancestor of this container.", nameof(module)); - current = current.Owner; - } - } - if (_children.Contains(module)) return; - module.Owner?.RemoveChild(module); + module.Owner?.RemoveChildInternal(module, false); module.Owner = this; module.Root = Root; _children.Insert(index, module); @@ -75,6 +61,11 @@ public abstract class DockContainer : DockModule /// /// The module to remove. public virtual void RemoveChild(DockModule module) + { + RemoveChildInternal(module, true); + } + + internal void RemoveChildInternal(DockModule module, bool triggerCleanup) { ArgumentNullException.ThrowIfNull(module); @@ -82,22 +73,28 @@ public abstract class DockContainer : DockModule { module.Owner = null; module.Root = null; - if (!_isCleaningUp) + if (!_isCleaningUp && triggerCleanup) { CheckCleanup(); } } } + /// + /// Replaces an existing child module with a new one. + /// + /// The child module to be replaced. + /// The new child module to insert. public virtual void ReplaceChild(DockModule oldChild, DockModule newChild) { ArgumentNullException.ThrowIfNull(oldChild); - ArgumentNullException.ThrowIfNull(newChild); + ValidateChild(newChild); + int index = _children.IndexOf(oldChild); if (index < 0) throw new ArgumentException("oldChild not found"); // Detach newChild from its current owner if any - newChild.Owner?.RemoveChild(newChild); + newChild.Owner?.RemoveChildInternal(newChild, false); // Remove oldChild without triggering cleanup _isCleaningUp = true; @@ -115,15 +112,41 @@ public abstract class DockContainer : DockModule { _isCleaningUp = false; } - OnChildrenUpdated(); + CheckCleanup(); } + /// + /// Checks if the container is empty and removes it from its owner if necessary. + /// protected virtual void CheckCleanup() { if (_children.Count == 0) { - Owner?.RemoveChild(this); + Owner?.RemoveChildInternal(this, true); + } + } + + /// + /// Validates if a module can be added as a child to this container. + /// + /// The module to validate. + protected virtual void ValidateChild(DockModule module) + { + ArgumentNullException.ThrowIfNull(module); + + if (module == this) + throw new ArgumentException("Cannot add a container to itself.", nameof(module)); + + if (module is DockContainer container) + { + var current = Owner; + while (current != null) + { + if (current == container) + throw new ArgumentException("Cannot add a container that is an ancestor of this container.", nameof(module)); + current = current.Owner; + } } } diff --git a/src/Editor/Ghost.Editor/View/Controls/Docking/DockGroup.cs b/src/Editor/Ghost.Editor/View/Controls/Docking/DockGroup.cs index d2a32bd..bc0f2f6 100644 --- a/src/Editor/Ghost.Editor/View/Controls/Docking/DockGroup.cs +++ b/src/Editor/Ghost.Editor/View/Controls/Docking/DockGroup.cs @@ -18,28 +18,14 @@ public partial class DockGroup : DockContainer DefaultStyleKey = typeof(DockGroup); } - public override void AddChild(DockModule module) + protected override void ValidateChild(DockModule module) { - ArgumentNullException.ThrowIfNull(module); + base.ValidateChild(module); if (module is not DockDocument) { throw new ArgumentException($"{nameof(DockGroup)} only accepts {nameof(DockDocument)} children.", nameof(module)); } - - base.AddChild(module); - } - - public override void InsertChild(int index, DockModule module) - { - ArgumentNullException.ThrowIfNull(module); - - if (module is not DockDocument) - { - throw new ArgumentException($"{nameof(DockGroup)} only accepts {nameof(DockDocument)} children.", nameof(module)); - } - - base.InsertChild(index, module); } protected override void OnApplyTemplate()