Weak scrub fix & TWeakMap

framework/Collections/IWeakCollection.php

@@ -13,8 +13,8 @@
 /**
  * IWeakCollection interface
  *
- * This is implemented by Weak Collection classes such as {@see \Prado\Collections\TWeakCallableCollection}
- * and {@see \Prado\Collections\TWeakList} for providing weak collections.
+ * This is implemented by Weak Collection classes such as {@see \Prado\Collections\TWeakCallableCollection},
+ * {@see \Prado\Collections\TWeakList}, and {@see \Prado\Collections\TWeakMap} for providing weak collections.
  *
  * @author Brad Anderson <belisoful@icloud.com>
  * @since 4.3.0

framework/Collections/TWeakCallableCollection.php

@@ -58,12 +58,6 @@ class TWeakCallableCollection extends TPriorityList implements IWeakCollection,
 {
 	use TWeakCollectionTrait;
 
-	/** @var ?bool Should invalid WeakReferences automatically be deleted from the list */
-	private ?bool $_discardInvalid = null;
-
-	/** @var int The number of TEventHandlers in the list */
-	private int $_eventHandlerCount = 0;
-
 	/**
 	 * Constructor.
 	 * Initializes the list with an array or an iterable object.
@@ -226,44 +220,56 @@ public static function filterItemForInput(&$handler, $validate = false): void
 	 * have lost their object.
 	 * All invalid WeakReference[s] are optionally removed from the list when {@see
 	 * getDiscardInvalid} is true.
+	 *
+	 * This method is re-entrancy safe via the {@see isScrubbing} guard. PHP's cyclic
+	 * garbage collector can fire between opcodes and invoke object destructors
+	 * ({@see TComponent::__destruct} → `unlisten()` → `remove()`) that call back into
+	 * this method while the outer iteration is still running over `_d[$priority]`. The
+	 * guard ensures that nested calls return immediately, preventing the inner call from
+	 * modifying the array that the outer loop is currently scrubbing.
 	 * @since 4.3.0
 	 */
 	protected function scrubWeakReferences()
 	{
-		if (!$this->getDiscardInvalid() || !$this->weakChanged()) {
+		if ($this->isScrubbing() || !$this->getDiscardInvalid() || !$this->weakChanged()) {
 			return;
 		}
-		foreach (array_keys($this->_d) as $priority) {
-			for ($c = $i = count($this->_d[$priority]), $i--; $i >= 0; $i--) {
-				$a = is_array($this->_d[$priority][$i]);
-				$isEventHandler = $weakRefInvalid = false;
-				$arrayInvalid = $a && is_object($this->_d[$priority][$i][0]) && ($this->_d[$priority][$i][0] instanceof WeakReference) && $this->_d[$priority][$i][0]->get() === null;
-				if (is_object($this->_d[$priority][$i])) {
-					$object = $this->_d[$priority][$i];
-					if ($isEventHandler = ($object instanceof TEventHandler)) {
-						$object = $object->getHandlerObject(true);
-					}
-					$weakRefInvalid = ($object instanceof WeakReference) && $object->get() === null;
-				}
-				if ($arrayInvalid || $weakRefInvalid) {
-					$c--;
-					$this->_c--;
-					if ($i === $c) {
-						array_pop($this->_d[$priority]);
-					} else {
-						array_splice($this->_d[$priority], $i, 1);
+		$this->setScrubbing(true);
+		try {
+			foreach (array_keys($this->_d) as $priority) {
+				for ($c = $i = count($this->_d[$priority]), $i--; $i >= 0; $i--) {
+					$a = is_array($this->_d[$priority][$i]);
+					$isEventHandler = $weakRefInvalid = false;
+					$arrayInvalid = $a && is_object($this->_d[$priority][$i][0]) && ($this->_d[$priority][$i][0] instanceof WeakReference) && $this->_d[$priority][$i][0]->get() === null;
+					if (is_object($this->_d[$priority][$i])) {
+						$object = $this->_d[$priority][$i];
+						if ($isEventHandler = ($object instanceof TEventHandler)) {
+							$object = $object->getHandlerObject(true);
+						}
+						$weakRefInvalid = ($object instanceof WeakReference) && $object->get() === null;
 					}
-					if ($isEventHandler) {
-						$this->_eventHandlerCount--;
+					if ($arrayInvalid || $weakRefInvalid) {
+						$c--;
+						$this->_c--;
+						if ($i === $c) {
+							array_pop($this->_d[$priority]);
+						} else {
+							array_splice($this->_d[$priority], $i, 1);
+						}
+						if ($isEventHandler) {
+							$this->_eventHandlerCount--;
+						}
 					}
 				}
+				if (!$c) {
+					unset($this->_d[$priority]);
+				}
 			}
-			if (!$c) {
-				unset($this->_d[$priority]);
-			}
+			$this->_fd = null;
+			$this->weakResetCount();
+		} finally {
+			$this->setScrubbing(false);
 		}
-		$this->_fd = null;
-		$this->weakResetCount();
 	}
 
 	/**
@@ -1089,8 +1095,5 @@ protected function _getZappableSleepProps(&$exprops)
 		$this->_c = $c;
 
 		$this->_weakZappableSleepProps($exprops);
-		if ($this->_discardInvalid === null) {
-			$exprops[] = "\0" . __CLASS__ . "\0_discardInvalid";
-		}
 	}
 }

framework/Collections/TWeakCollectionTrait.php

@@ -43,6 +43,37 @@ trait TWeakCollectionTrait
 	/** @var int Number of known objects in the collection. */
 	private int $_weakCount = 0;
 
+	/** @var bool Re-entrancy guard for {@see scrubWeakReferences}: true while executing. */
+	private bool $_scrubbing = false;
+
+	/** @var ?bool Whether GC'd entries are automatically discarded. Null = lazy init. */
+	private ?bool $_discardInvalid = null;
+
+	/** @var int Number of {@see \Prado\TEventHandler} instances currently tracked. */
+	private int $_eventHandlerCount = 0;
+
+	/**
+	 * Returns true if {@see scrubWeakReferences} is currently executing.
+	 * The re-entrancy guard prevents cyclic GC from invoking a second scrub
+	 * while the outer loop is still iterating.
+	 * @return bool
+	 * @since 4.3.3
+	 */
+	protected function isScrubbing(): bool
+	{
+		return $this->_scrubbing;
+	}
+
+	/**
+	 * Sets or clears the {@see scrubWeakReferences} re-entrancy guard.
+	 * @param bool $value True when entering the scrub loop; false on exit.
+	 * @since 4.3.3
+	 */
+	protected function setScrubbing(bool $value): void
+	{
+		$this->_scrubbing = $value;
+	}
+
 	/**
 	 * Initializes a new WeakMap
 	 */
@@ -163,11 +194,29 @@ protected function weakStop(): void
 	}
 
 	/**
+	 * Appends weak-collection properties that must be excluded from serialization.
+	 *
+	 * Always excluded (pure runtime state):
+	 *  - `_weakMap` / `_weakCount` — rebuilt from data on wakeup
+	 *  - `_scrubbing` — transient re-entrancy flag, always false at rest
+	 *  - `_eventHandlerCount` — derived from data, always 0 after wakeup
+	 *
+	 * Conditionally excluded:
+	 *  - `_discardInvalid` when null — null means "lazy: derive from ReadOnly",
+	 *    so persisting null is harmless but storing the derived value is preferable;
+	 *    excluding null causes wakeup to re-derive correctly.
+	 *
 	 * @param array &$exprops Properties to remove from serialize.
+	 * @since 4.3.0
 	 */
 	protected function _weakZappableSleepProps(array &$exprops): void
 	{
 		$exprops[] = "\0" . __CLASS__ . "\0_weakMap";
 		$exprops[] = "\0" . __CLASS__ . "\0_weakCount";
+		$exprops[] = "\0" . __CLASS__ . "\0_scrubbing";
+		$exprops[] = "\0" . __CLASS__ . "\0_eventHandlerCount";
+		if ($this->_discardInvalid === null) {
+			$exprops[] = "\0" . __CLASS__ . "\0_discardInvalid";
+		}
 	}
 }

framework/Collections/TWeakList.php

@@ -67,14 +67,6 @@ class TWeakList extends TList implements IWeakCollection, ICollectionFilter
 {
 	use TWeakCollectionTrait;
 
-	/** @var ?bool Should invalid WeakReference automatically be deleted from the list.
-	 *    Default True.
-	 */
-	private ?bool $_discardInvalid = null;
-
-	/** @var int The number of TEventHandlers in the list */
-	private int $_eventHandlerCount = 0;
-
 	/**
 	 * Constructor.
 	 * Initializes the weak list with an array or an iterable object.
@@ -195,32 +187,45 @@ public static function filterItemForInput(&$item): void
 
 	/**
 	 * When a change in the WeakMap is detected, scrub the list of invalid WeakReference.
+	 *
+	 * Re-entrancy guard: PHP's cyclic garbage collector can fire between any two opcodes
+	 * and invoke object destructors mid-loop.  If a destructor removes an entry from this
+	 * list (e.g. via TComponent::__destruct → unlisten → remove → scrubWeakReferences),
+	 * the inner call would shorten $_d and $_c while the outer loop is still iterating,
+	 * making the outer index stale and causing an "Undefined array key" error.  The
+	 * {@see isScrubbing} guard prevents the inner call from executing; any entries the
+	 * inner call would have removed are picked up during the next outer pass.
 	 */
 	protected function scrubWeakReferences(): void
 	{
-		if (!$this->getDiscardInvalid() || !$this->weakChanged()) {
+		if ($this->isScrubbing() || !$this->getDiscardInvalid() || !$this->weakChanged()) {
 			return;
 		}
-		for ($i = $this->_c - 1; $i >= 0; $i--) {
-			if (is_object($this->_d[$i])) {
-				$object = $this->_d[$i];
-				if ($isEventHandler = ($object instanceof TEventHandler)) {
-					$object = $object->getHandlerObject(true);
-				}
-				if (($object instanceof WeakReference) && $object->get() === null) {
-					$this->_c--;
-					if ($i === $this->_c) {
-						array_pop($this->_d);
-					} else {
-						array_splice($this->_d, $i, 1);
+		$this->setScrubbing(true);
+		try {
+			for ($i = $this->_c - 1; $i >= 0; $i--) {
+				if (is_object($this->_d[$i])) {
+					$object = $this->_d[$i];
+					if ($isEventHandler = ($object instanceof TEventHandler)) {
+						$object = $object->getHandlerObject(true);
 					}
-					if ($isEventHandler) {
-						$this->_eventHandlerCount--;
+					if (($object instanceof WeakReference) && $object->get() === null) {
+						$this->_c--;
+						if ($i === $this->_c) {
+							array_pop($this->_d);
+						} else {
+							array_splice($this->_d, $i, 1);
+						}
+						if ($isEventHandler) {
+							$this->_eventHandlerCount--;
+						}
 					}
 				}
 			}
+			$this->weakResetCount();
+		} finally {
+			$this->setScrubbing(false);
 		}
-		$this->weakResetCount();
 	}
 
 	/**
@@ -627,8 +632,5 @@ protected function _getZappableSleepProps(&$exprops)
 		$this->_c = $c;
 
 		$this->_weakZappableSleepProps($exprops);
-		if ($this->_discardInvalid === null) {
-			$exprops[] = "\0" . __CLASS__ . "\0_discardInvalid";
-		}
 	}
 }