merge: cascade 8.4 into master - #225
Merged
Merged
Conversation
Every persistent structure z-engine mints allocated through a hardcoded Core::trackedNew(..., persistent) - FFI's pemalloc, i.e. the process heap - which a consumer that keeps its objects in a fork-shared mmap arena cannot redirect. The new ZEngine\Memory\Allocator interface is that seam: it hands out ADDRESSES of zeroed, aligned blocks (no FFI\CData crosses the boundary, so an arena implementation in a consumer package binds its own mmap primitives and returns plain integers) and reports whether it keeps ownership of what it hands out. EngineAllocator is the default implementation and reproduces the three allocation shapes the framework used to hardcode - tracked persistent, tracked request-lifetime and untracked persistent - so a caller that passes nothing gets byte-for-byte the behavior it had before. HashTable's constructor takes the optional allocator and refuses destroy() when the memory belongs to a foreign allocator: both frees assume z-engine's own allocator and would corrupt an arena. PersistentHashTable can now be handed the OTHER half of a table's memory: installExternalStorage() writes the engine's own zend_hash_real_init_mixed layout (hash part of two uint32_t slots per bucket reset to HT_INVALID_IDX, buckets right behind it) over a caller-allocated block, before the first insert makes the engine allocate storage of its own. Because the engine grows a full table by perealloc()ing that very block, growth is guarded from both sides: inserts through the wrapper refuse the write that would trigger the resize, and assertNoGrowth() diagnoses a relocation that engine paths caused behind the wrapper's back. Refs #223 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B9xaBchjdo1atarNZ6sZqe
The three primitives every persistent graph is built from - the object clones of PersistentObjectFactory, the interned blocks of StringEntry::persistent(Interned) and the tables of PersistentGraphCloner - now take an optional allocator and pass it on, so a whole graph can be cloned into memory the caller owns. Passing nothing keeps each primitive on its own historical default, which is deliberately not the same allocator for all of them: object clones and table structs stay tracked malloc blocks, string blocks stay untracked ones, exactly as before. Core::offsetOfField() is the named form of the type(...)->getStructFieldOffset(...) pair the string minting needed - the same remedy sizeOfType() is for sizeof(type(...)), and one raw FFI\CType less in the API surface. Covered by an arena-shaped test allocator (bump pointer over one owned region, reporting ownership): clones, strings and whole cyclic graphs land inside the region, nothing of them reaches z-engine's block registry, and a table built on it refuses to be freed. The external bucket storage is covered end to end - layout math against HT_SIZE_EX, lookups through the installed hash part, the capacity guard at the resize boundary, upserts still allowed when full, and the relocation diagnosis. Refs #223 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B9xaBchjdo1atarNZ6sZqe
…s need Packages built on z-engine had to reach through Core::$executor for two operations that have no public equivalent: registering an object the engine did not allocate (and handing its slot back), and probing the engine class table for a class name. Both are core-layer state per AGENTS.md, and the remedy for that is a named public method here rather than a reach-through there. ObjectStore::current() is the public entry point into EG(objects_store); register()/unregister() are the CData-free, ownership-explicit forms of put()/recycle() - the store never takes ownership, so the caller keeps the object memory and gives the slot back before releasing it. ReflectionClass::fromClassTable() answers "is this class already in the engine?" without autoloading and without throwing, which is what a re-attachment path needs before it trusts recorded class metadata. Refs #223 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B9xaBchjdo1atarNZ6sZqe
…eview ObjectStore::current() re-exposed Core::$executor->objectStore through a side door, and the register()/unregister() pair was only reachable through it - the trio recreated the very reach-through it was meant to remove, with an extra hop. The operations belong to the object being registered, not to a store handed out to callers. ObjectEntry::register() registers THIS object in the current request's store and returns the handle the engine assigned; unregister() reads the object's handle at call time and returns that slot to the free list. The slot is verified to actually hold this object first, so a stale handle - never registered, already unregistered, or a slot meanwhile reused - is refused with a named domain exception instead of silently detaching somebody else's object. ObjectStore is byte-for-byte what it was before this PR: put() and recycle() stay @internal, and a consumer never touches EG(objects_store) at all. Refs #223 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B9xaBchjdo1atarNZ6sZqe
The registration test released the clone's memory while a value materialized
from it was still in scope. That value is a real PHP alias of the very same
zend_object, so destroying it at the end of the test method made the engine
write a refcount into a block that had already been handed back to malloc -
and gc.refcount sits at offset 0 of a zend_object, which is exactly where
glibc keeps the tcache next pointer of a free chunk. The next persistent
clone of the same size popped that chunk and the allocator aborted, far away
from the mistake:
malloc(): unaligned tcache chunk detected (SIGABRT, exit 134)
Repro (deterministic, aborts during the SECOND test of the class - the one
whose clone lands in the poisoned size class):
MALLOC_CHECK_=3 MALLOC_PERTURB_=85 php8.4 -d ffi.enable=1 \
-d opcache.jit=off vendor/bin/phpunit --filter ObjectEntryRegistration
The registration path itself is sound: unregister() bounds-checks the handle
against the store top and validates the bucket through the store's own
tagged-pointer check before it compares identity, so it never dereferences a
free-list sentinel. What was missing is the other half of the ownership
contract, now spelled out on register(): registration makes the object
reachable from userland, and every alias materialized from it must be gone
before the memory is released, not just the store slot.
Refs #223
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B9xaBchjdo1atarNZ6sZqe
…ovsz feat(memory): allocator seam for persistent cloning + external arData install (closes #223)
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
2 participants
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.
Automated cascade merge of
8.4intomaster(branch flow defined in.github/branch-flow.json).Resolve conflicts in favour of the newer engine structures where they touch
include/- regenerate headers on the target branch instead of merging them textually. See AGENTS.md.