Fix SplFixedArray leak when re-initialised from a destructor

setSize(0) clears the array before destroying its elements, so it looks
unconstructed to userland. __construct(), __wakeup() and __unserialize()
then re-initialised it, and the in-progress clear discarded the buffer
they had installed.

cb3dc62 fixed the same leak for a re-entrant setSize() by testing the
resize sentinel first; apply that test to the other three entry points.

Author: marc-mabe

NEWS

@@ -28,6 +28,8 @@ PHP                                                                        NEWS
 - SPL:
   . Fixed SplFixedArray::setSize() doing nothing on subclasses whose
     constructor does not call parent::__construct(). (Marc Bennewitz)
+  . Fixed memory leak when __construct(), __wakeup() or __unserialize() is
+    called from an element destructor during setSize(0). (Marc Bennewitz)
 
 - Zip:
   . Fixed ZipArchive::extractTo() ignoring files given in a non-list array.

ext/spl/spl_fixedarray.c

@@ -77,6 +77,14 @@ static bool spl_fixedarray_empty(spl_fixedarray *array)
 	return true;
 }
 
+/* True while spl_fixedarray_resize() runs. A clear empties the array before
+ * destroying its elements, so emptiness alone cannot tell "never constructed"
+ * from "clear in progress"; re-initialising in that window leaks. */
+static bool spl_fixedarray_resize_in_progress(const spl_fixedarray *array)
+{
+	return array->cached_resize >= 0;
+}
+
 static void spl_fixedarray_default_ctor(spl_fixedarray *array)
 {
 	array->size = 0;
@@ -188,9 +196,9 @@ static void spl_fixedarray_resize(spl_fixedarray *array, zend_long size)
 
 	/* clearing the array */
 	if (size == 0) {
+		/* Clears elements and size; resetting them afterwards would leak
+		 * anything a destructor re-installed. */
 		spl_fixedarray_dtor(array);
-		array->elements = NULL;
-		array->size = 0;
 	} else if (size > array->size) {
 		array->elements = safe_erealloc(array->elements, size, sizeof(zval), 0);
 		spl_fixedarray_init_elems(array, array->size, size);
@@ -201,8 +209,12 @@ static void spl_fixedarray_resize(spl_fixedarray *array, zend_long size)
 		array->elements = erealloc(array->elements, sizeof(zval) * size);
 	}
 
-	/* If resized within the destructor, take the last resize command and perform it */
+	/* If resized within the destructor, take the last resize command and
+	 * perform it. The sentinel is still set: re-initialising during a
+	 * resize is refused. */
 	zend_long cached_resize = array->cached_resize;
+	ZEND_ASSERT(cached_resize >= 0);
+
 	array->cached_resize = -1;
 	if (cached_resize != size) {
 		spl_fixedarray_resize(array, cached_resize);
@@ -557,7 +569,7 @@ PHP_METHOD(SplFixedArray, __construct)
 
 	intern = Z_SPLFIXEDARRAY_P(object);
 
-	if (!spl_fixedarray_empty(&intern->array)) {
+	if (UNEXPECTED(!spl_fixedarray_empty(&intern->array) || spl_fixedarray_resize_in_progress(&intern->array))) {
 		/* called __construct() twice, bail out */
 		return;
 	}
@@ -575,7 +587,7 @@ PHP_METHOD(SplFixedArray, __wakeup)
 		RETURN_THROWS();
 	}
 
-	if (intern->array.size == 0) {
+	if (EXPECTED(intern->array.size == 0 && !spl_fixedarray_resize_in_progress(&intern->array))) {
 		int index = 0;
 		int size = zend_hash_num_elements(intern_ht);
 
@@ -637,7 +649,7 @@ PHP_METHOD(SplFixedArray, __unserialize)
 		RETURN_THROWS();
 	}
 
-	if (intern->array.size == 0) {
+	if (EXPECTED(intern->array.size == 0 && !spl_fixedarray_resize_in_progress(&intern->array))) {
 		size = zend_hash_num_elements(data);
 		spl_fixedarray_init_non_empty_struct(&intern->array, size);
 		if (!size) {

ext/spl/tests/SplFixedArray_setSize_destruct_reinit_during_clear.phpt

@@ -0,0 +1,68 @@
+--TEST--
+SplFixedArray::setSize: re-initialising from a destructor during clear (GH-23811)
+--DESCRIPTION--
+setSize(0) clears elements and size before running the element destructors, so
+the array momentarily looks like it was never constructed. __construct(),
+__wakeup() and __unserialize() must not re-initialise it in that window: the
+in-progress clear would discard whatever they installed, leaking it.
+--FILE--
+<?php
+class Reentrant {
+    public static $arr = null;
+    public static $action = null;
+    public function __destruct() {
+        if (self::$action === null) {
+            return;
+        }
+        $fn = self::$action;
+        self::$action = null;
+        $fn(self::$arr);
+    }
+}
+
+function clear_with(callable $action): void {
+    $arr = new SplFixedArray(2);
+    $arr[0] = new Reentrant();
+    $arr[1] = "tail";
+    Reentrant::$arr = $arr;
+    Reentrant::$action = $action;
+
+    $arr->setSize(0);
+    echo "size: ", $arr->getSize(), "\n";
+
+    /* The array must still be usable. */
+    $arr->setSize(1);
+    $arr[0] = "ok";
+    var_dump($arr[0]);
+
+    Reentrant::$arr = null;
+    Reentrant::$action = null;
+}
+
+echo "-- __construct() --\n";
+clear_with(function ($arr) { $arr->__construct(5); });
+
+/* __construct() is ignored, but the following setSize() is still recorded as
+ * the pending resize and applied once the clear finishes. */
+echo "-- __construct() then setSize() --\n";
+clear_with(function ($arr) { $arr->__construct(7); $arr->setSize(3); });
+
+echo "-- __unserialize() --\n";
+clear_with(function ($arr) { $arr->__unserialize(["a", "b", "c"]); });
+
+echo "-- __wakeup() --\n";
+clear_with(function ($arr) { @$arr->__wakeup(); });
+?>
+--EXPECT--
+-- __construct() --
+size: 0
+string(2) "ok"
+-- __construct() then setSize() --
+size: 3
+string(2) "ok"
+-- __unserialize() --
+size: 0
+string(2) "ok"
+-- __wakeup() --
+size: 0
+string(2) "ok"