#161: carry closure scope/called_scope by name across thread transfer#163
Merged
Conversation
A transferred closure lost its class scope: a static closure declared inside a class arrived in the worker unscoped (first self::/static:: threw "Cannot access self"), and a $this-bound closure got Z_OBJCE($this) instead of its declaring class, breaking self:: and private visibility under inheritance. The snapshot now stores scope and called_scope as persistent name strings; async_thread_create_closure re-resolves them in the target thread via zend_lookup_class (autoload-aware). A missing class is a hard error instead of a silently unscoped closure. Closures scoped to anonymous classes are rejected at transfer time. Bootloaders are deliberately copied unscoped: they bootstrap a thread where their own declaring class may not exist yet (chicken-and-egg). Applies to both the spawn_thread bootloader slot and the pool bootloader stored in the entry slot (entry_is_bootloader). thread_call_closure now passes called_scope as the frame's object_or_scope for scoped static calls (zend_vm_init_call_frame asserts otherwise) and skips execution when closure creation threw. The closure_transfer_obj LOAD branch follows the default-loader convention on failure: throw + stdClass fallback.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Worker payloads are lexically declared inside StandardSteps, but chaos workers boot bare (no harness classes). With strict scope transfer the class would now be required on the worker side; strip the scope via Closure::bind(fn, null, null) — the payloads never touch self::/static::.
Same contract as the spawn_thread payloads in the previous commit: pool workers boot bare, so harness-scoped task closures must be stripped of their lexical class scope before transfer.
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
1 participant
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.
Fixes the scope half of #161.
A transferred closure lost its class scope: a
staticclosure declared inside a class arrived in the worker unscoped, so its firstself::/static::threwCannot access "self" when no class scope is active— and the worker died silently. A$this-bound closure gotZ_OBJCE($this)instead of its declaring class, breakingself::and private-member visibility under inheritance.Changes
scope/called_scopeas persistent name strings;async_thread_create_closurere-resolves them in the target thread viazend_lookup_class(autoload-aware).Cannot restore closure scope: class "X" not found in the target thread— no silently unscoped closure.thread_call_closurepassescalled_scopeas the frame'sobject_or_scopefor scoped static calls and skips execution when creation threw.closure_transfer_objLOAD failure follows the default-loader convention: throw + stdClass fallback.Tests
tests/thread/072— issue repro: static closure withself::/static::throughspawn_thread.tests/thread/073— scope class missing in worker → clear error.tests/thread/057— re-pinned to native scope semantics (worker: base), as its description anticipated.The deadlock half of #161 (parent parked forever on a
ThreadChannelafter silent worker death) is a separate problem shared with #162 and is not addressed here.