fix: preserve a closure's bound $this across thread transfer#123
Merged
Conversation
closure_transfer_obj() built the transfer snapshot from a zend_fcall_t with only fci_cache.function_handler set. thread_copy_callable() takes the bound instance from fci_cache.object, so leaving it NULL snapshotted the closure unbound: the destination thread recreated an object-less closure, and a method/object-bound closure then dereferenced a NULL $this — SIGSEGV on the first $this access. Populate fci_cache.object from the closure's this_ptr before snapshotting; the snapshot/recreate machinery already handles bound_this once given the instance. Affects the generic closure-transfer path only — a $this-bound closure travelling as a bound variable, array element, argument or channel message. spawn_thread / ThreadPool tasks were unaffected: they resolve fci_cache.object themselves. Adds tests/thread/058-spawn_thread_captured_bound_closure.phpt.
EdmondDantes
added a commit
to true-async/server
that referenced
this pull request
May 21, 2026
The regression test depends on true-async/php-async#123; until that fix is in the CI PHP build a $this-bound handler closure loses its binding across the worker transfer and the test fails. XFAIL keeps CI green in both states (expected-fail before the fix, pass after); the section is to be removed once php-async#123 is merged.
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.
Bug
A
$this-bound closure transferred to another thread through the generic closure-transfer path arrives unbound. Invoking a method- or object-bound closure in the destination thread then dereferences aNULL$this— SIGSEGV on the first$thisaccess.Reproduced end-to-end: an object-bound HTTP handler registered on
TrueAsync\HttpServerwithsetWorkers(N>1)segfaults every worker on the first request. A directAsync\ThreadPoolsubmit of an object-bound closure is not affected — the difference led straight to the cause.Cause
closure_transfer_obj()(thetransfer_objhandler installed onClosure) builds the transfer snapshot like this:Only
fci_cache.function_handleris set. Butthread_copy_callable()reads the bound instance fromfci_cache.object:So
bound_thisstaysUNDEF, andasync_thread_create_closure()recreates an unbound closure.spawn_thread/ThreadPooltasks are unaffected because they hand the snapshot azend_fcall_twhosefci_cache.objectis already resolved. The defect is specific toclosure_transfer_obj— the path taken when a closure travels as a bound variable, array element, argument, or channel message (or via an extension'sZEND_ASYNC_THREAD_TRANSFER_ZVAL).Fix
Populate
fci_cache.objectfrom the closure'sthis_ptrbefore snapshotting. The snapshot/recreate machinery already handlesbound_thisend to end once it is given the instance — no other change needed.Test
tests/thread/058-spawn_thread_captured_bound_closure.phpt— a$this-bound closure captured as a bound variable of aspawn_threadtask (forcing the generic path), invoked in the worker; also asserts the binding is a deep copy. Fails (NULL-$thisderef) before the fix, passes after.