Component: attached handles are called top-down (ancestor → descendant) (BC break)

Implementation handles tree mutations during listener execution:
- Listeners can modify tree (remove self, siblings, parent)
- Validity check before processing children
- Deduplication prevents calling same listener twice
- Reentry guard prevents infinite loops

Author: dg

src/ComponentModel/Component.php

@@ -32,9 +32,6 @@ abstract class Component implements IComponent
 	 */
 	private array $monitors = [];
 
-	/** Prevents nested listener execution during refreshMonitors */
-	private bool $callingListeners = false;
-
 
 	/**
 	 * Finds the closest ancestor of specified type.
@@ -199,35 +196,25 @@ protected function validateParent(IContainer $parent): void
 
 
 	/**
-	 * Refreshes monitors.
-	 * @param  ?array<string, true>  $missing  (array = attaching, null = detaching)
-	 * @param  array<int, array{?\Closure, IComponent}>  $listeners
+	 * Refreshes monitors when attaching/detaching from component tree.
+	 * @param  ?array<string, true>  $missing  null = detaching, array = attaching
+	 * @param  array<array{\Closure, int}>  $called  deduplication tracking
+	 * @param  array<int, true>  $processed  prevents reentry
 	 */
-	private function refreshMonitors(int $depth, ?array &$missing = null, array &$listeners = []): void
+	private function refreshMonitors(
+		int $depth,
+		?array &$missing = null,
+		array &$called = [],
+		array &$processed = [],
+	): void
 	{
-		if ($this instanceof IContainer) {
-			foreach ($this->getComponents() as $component) {
-				if ($component instanceof self) {
-					$component->refreshMonitors($depth + 1, $missing, $listeners);
-				}
-			}
+		$componentId = spl_object_id($this);
+		if (isset($processed[$componentId])) {
+			return; // Prevent reentry on same component (can happen if listener modifies tree)
 		}
+		$processed[$componentId] = true;
 
-		if ($missing === null) { // detaching
-			foreach ($this->monitors as $type => [$ancestor, $inDepth, , $callbacks]) {
-				if (isset($inDepth) && $inDepth > $depth) { // only process if ancestor was deeper than current detachment point
-					assert($ancestor !== null);
-					if ($callbacks) {
-						$this->monitors[$type] = [null, null, null, $callbacks];
-						foreach ($callbacks as [, $detached]) {
-							$listeners[] = [$detached, $ancestor];
-						}
-					} else { // no listeners, just cached lookup result - clear it
-						unset($this->monitors[$type]);
-					}
-				}
-			}
-		} else { // attaching
+		if ($missing !== null) { // attaching
 			foreach ($this->monitors as $type => [$ancestor, , , $callbacks]) {
 				if (isset($ancestor)) { // already cached and valid - skip
 					continue;
@@ -243,7 +230,10 @@ private function refreshMonitors(int $depth, ?array &$missing = null, array &$li
 					assert($type !== '');
 					if ($ancestor = $this->lookup($type, throw: false)) {
 						foreach ($callbacks as [$attached]) {
-							$listeners[] = [$attached, $ancestor];
+							if ($attached && !in_array($key = [$attached, spl_object_id($ancestor)], $called, false)) {
+								$attached($ancestor);
+								$called[] = $key; // Deduplicate: same callback + same object = call once
+							}
 						}
 					} else {
 						$missing[$type] = true; // ancestor not found - remember so we don't check again
@@ -254,19 +244,33 @@ private function refreshMonitors(int $depth, ?array &$missing = null, array &$li
 			}
 		}
 
-		if ($depth === 0 && !$this->callingListeners) { // call listeners
-			$this->callingListeners = true;
-			try {
-				$called = [];
-				foreach ($listeners as [$callback, $component]) {
-					$key = [$callback, spl_object_id($component)];
-					if ($callback && !in_array($key, $called, strict: false)) { // deduplicate: same callback + same object = call once
-						$callback($component);
-						$called[] = $key;
+		if ($this instanceof IContainer) {
+			foreach ($this->getComponents() as $component) {
+				if ($component instanceof self
+					&& !isset($processed[spl_object_id($component)]) // component may have been processed already
+					&& $component->getParent() === $this  // may have been removed by previous sibling's listener
+				) {
+					$component->refreshMonitors($depth + 1, $missing, $called, $processed);
+				}
+			}
+		}
+
+		if ($missing === null) { // detaching
+			foreach ($this->monitors as $type => [$ancestor, $inDepth, , $callbacks]) {
+				if (isset($inDepth) && $inDepth > $depth) { // only process if ancestor was deeper than current detachment point
+					assert($ancestor !== null);
+					if ($callbacks) {
+						$this->monitors[$type] = [null, null, null, $callbacks]; // clear cached object, keep listener registrations
+						foreach ($callbacks as [, $detached]) {
+							if ($detached && !in_array($key = [$detached, spl_object_id($ancestor)], $called, false)) {
+								$detached($ancestor);
+								$called[] = $key; // Deduplicate: same callback + same object = call once
+							}
+						}
+					} else { // no listeners, just cached lookup result - clear it
+						unset($this->monitors[$type]);
 					}
 				}
-			} finally {
-				$this->callingListeners = false;
 			}
 		}
 	}

tests/ComponentModel/Component.monitor.phpt

@@ -88,8 +88,8 @@ test('monitor with handlers', function () {
 	$a = new A;
 	$a['b'] = $b;
 	Assert::same([
-		'ATTACHED(A, C)',
 		'ATTACHED(A, B)',
+		'ATTACHED(A, C)',
 	], $log);
 
 	$log = [];

tests/ComponentModel/Component.monitorEdgeCases.phpt

@@ -79,7 +79,7 @@ test('parent removes children before their listeners run', function () {
 	$root->addComponent($parent, 'parent');
 
 	Assert::true($parent->attachedCalled);
-	Assert::true($child->attachedCalled); // problem: child listener is called although parent removed it first
+	Assert::false($child->attachedCalled, 'Child listener NOT called - parent removed it first');
 	Assert::same(0, count($parent->getComponents()));
 });
 
@@ -104,7 +104,7 @@ test('multiple nested children removed by parent', function () {
 	$root->addComponent($parent, 'parent');
 
 	foreach ($children as $i => $child) {
-		Assert::true($child->attachedCalled); // problem: child listener is called although parent removed it first
+		Assert::false($child->attachedCalled, "Child $i listener NOT called - parent removed it first");
 	}
 
 	Assert::same(0, count($parent->getComponents()));
@@ -131,7 +131,7 @@ test('child removes sibling before its listener runs', function () {
 	$root->addComponent($container, 'container');
 
 	Assert::true($child1->attachedCalled);
-	Assert::true($child2->attachedCalled); // problem: child2 listener is called although child1 removed it first
+	Assert::false($child2->attachedCalled, 'child2 listener NOT called - child1 removed it first');
 	Assert::same(1, count($container->getComponents()));
 });
 

tests/ComponentModel/Container.clone.monitor.phpt

@@ -186,8 +186,8 @@ test('reattach cloned component', function () {
 	$a['dolly'] = $dolly;
 
 	Assert::same([
-		'ATTACHED(A, C)',
 		'ATTACHED(A, B)',
+		'ATTACHED(A, C)',
 	], $log);
 
 	Assert::same([