From f984f95f42715560233a42470aef6a63fc24daa9 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Thu, 20 Aug 2026 13:36:14 +0000 Subject: [PATCH] fix(thread): a literal array may hold a hole; static_variables may not (#260) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit op_array_emalloc_copy_array rebuilds two different tables, and the invariant it asserted holds for one of them. Static variables need every bucket kept, because BIND_LEXICAL reaches a captured variable by a byte offset into arData and a dropped slot shifts every offset past it. A literal array carries no such promise: [0 => 'a', 2 => 'b'] compiles to a packed hash whose slot 1 is UNDEF, so nNumUsed is one past the element count and every debug build aborted when a task closure held one. The rebuild itself was already right — it iterates by key, and the keys are what the VM reads — so the assert moves to the caller whose offsets depend on it. Test 103-task_sparse_literal_array, which aborts without this. --- .../103-task_sparse_literal_array.phpt | 42 +++++++++++++++++++ thread.c | 24 +++++++---- 2 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 tests/thread_pool/103-task_sparse_literal_array.phpt diff --git a/tests/thread_pool/103-task_sparse_literal_array.phpt b/tests/thread_pool/103-task_sparse_literal_array.phpt new file mode 100644 index 0000000..00a5ff6 --- /dev/null +++ b/tests/thread_pool/103-task_sparse_literal_array.phpt @@ -0,0 +1,42 @@ +--TEST-- +ThreadPool: a task closure carrying a literal array with a gap in its keys +--SKIPIF-- + +--FILE-- + 'a', 2 => 'b'] compiles to a packed hash whose slot 1 is UNDEF, so its + * nNumUsed is one more than its element count. The op_array copy that hands a + * closure to a worker rebuilds literal arrays by key and skips that slot; the + * invariant it used to assert belongs to static_variables, where BIND_LEXICAL + * reads a byte offset into arData, and not to a literal. */ + +use Async\ThreadPool; +use function Async\spawn; +use function Async\await; + +spawn(function() { + $pool = new ThreadPool(1); + + $future = $pool->submit(function () { + $sparse = [0 => 'a', 2 => 'b']; + $nested = [1 => [3 => 'x'], 4 => 'y']; + $strkeys = ['k' => 1, 'l' => 2]; + + return implode(',', $sparse) + . '|' . $nested[1][3] . $nested[4] + . '|' . array_sum($strkeys) + . '|' . implode(',', array_keys($sparse)); + }); + + echo await($future), "\n"; + + $pool->close(); + echo "Done\n"; +}); +?> +--EXPECT-- +a,b|xy|3|0,2 +Done diff --git a/thread.c b/thread.c index 5e21b64..41babed 100644 --- a/thread.c +++ b/thread.c @@ -2332,15 +2332,17 @@ static zend_string *op_array_emalloc_copy_string(const zend_string *src) /* Rebuilds a literal array or the static variables of a nested definition. * - * The bucket order carries meaning for the second of those: BIND_LEXICAL - * addresses a captured variable by a byte offset into arData, computed when the - * closure was compiled. The rebuild keeps the order but cannot keep a hole, and - * the compiler leaves none — it seeds every slot — which is what the assert - * states. */ + * The rebuild keeps the bucket order, which carries meaning for the second of + * those: BIND_LEXICAL addresses a captured variable by a byte offset into + * arData, computed when the closure was compiled. It cannot keep a hole — the + * iteration skips an UNDEF slot — so the caller that needs the offsets asserts + * there is none; see the static_variables branch of op_array_to_emalloc. + * + * A literal array may hold one: [0 => 'a', 2 => 'b'] compiles to a packed hash + * whose slot 1 is UNDEF. Rebuilding it by key gives the same array back, since + * the keys are what the VM reads, so the hole costs nothing here. */ static HashTable *op_array_emalloc_copy_array(const HashTable *src) { - ZEND_ASSERT(zend_hash_num_elements(src) == src->nNumUsed); - HashTable *dst = zend_new_array(zend_hash_num_elements(src)); zend_string *key; @@ -2708,8 +2710,14 @@ static void op_array_to_emalloc(zend_op_array *op_array) } /* Nested defs carry the snapshot's persistent immutable static_variables - * (refcount 2); dup into a worker-local array destroy_op_array can free. */ + * (refcount 2); dup into a worker-local array destroy_op_array can free. + * + * BIND_LEXICAL reaches a captured variable by a byte offset into arData, + * so a rebuild that dropped a slot would shift every offset past it. The + * compiler seeds every slot and leaves no hole, which is what this states. */ if (op_array->static_variables) { + ZEND_ASSERT(zend_hash_num_elements(op_array->static_variables) + == op_array->static_variables->nNumUsed); op_array->static_variables = op_array_emalloc_copy_array(op_array->static_variables); }