A literal array may hold a hole; static_variables may not (#260) - #261
Merged
Conversation
#260) 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.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #260.
op_array_emalloc_copy_arrayrebuilds two different tables and asserted aninvariant that holds for one of them.
Static variables need every bucket kept:
BIND_LEXICALreaches a capturedvariable by a byte offset into
arData, computed at compile time, so a rebuildthat dropped a slot would shift every offset past it. The compiler seeds every
slot there, and the assert is right — it just belonged to that caller.
A literal array carries no such promise.
[0 => 'a', 2 => 'b']compiles to apacked hash whose slot 1 is UNDEF, so
nNumUsedis 3 while the element count is2, and every debug build aborted the moment a task closure held one:
The rebuild itself was already correct for that case — it iterates by key, and
the keys are what the VM reads back — so this moves the assert to the
static_variablesbranch ofop_array_to_emallocand leaves the literal pathalone.
Reproduction
Twelve lines, no server involved:
Aborts on
main, printsa,bwith this patch. It istests/thread_pool/103-task_sparse_literal_array.phpt, which also covers a nested sparse array and string keys beside it.What it was breaking
Every debug CI job of
true-async/serversince the merges of #254/#255/#256earlier today, through
tests/phpt/websocket/054-topics-pool-unsubscribe.phpt—a pool worker loading a handler closure. Server commit
5392ff7was green at09:11 UTC and the same commit failed at 12:50 UTC with no change on its side.
ext/async/tests: 1104 passed, 0 failed.true-async/servertests/phptagainst this build: 390 passed, 0 failed, both on a debug ZTS build.