feat: persistent-zval helpers (deep-copy zval trees across threads)#2366
Open
nicolas-grekas wants to merge 1 commit intophp:mainfrom
Open
feat: persistent-zval helpers (deep-copy zval trees across threads)#2366nicolas-grekas wants to merge 1 commit intophp:mainfrom
nicolas-grekas wants to merge 1 commit intophp:mainfrom
Conversation
Second step of the split suggested in php#2287: land the persistent-zval subsystem as a standalone, reviewable header, independent of background workers. This is the subsystem most likely to hide latent refcount or memory-lifetime bugs; reviewing it in isolation is higher-signal than finding issues inside a 3k-line diff. ## What - persistent_zval.h (renamed from the bg_worker_vars.h draft, prefix dropped for generality): - persistent_zval_validate: whitelist (scalars, arrays of allowed values, enum instances). Everything else fails fast. - persistent_zval_persist: deep-copy request -> persistent (pemalloc) memory. Fast paths baked in: interned strings shared, opcache- immutable arrays passed by pointer without copying or owning. - persistent_zval_free: deep-free; skips interned strings and immutable arrays (borrowed, not owned). - persistent_zval_to_request: deep-copy persistent -> fresh request memory. Enums re-resolved by class + case name on each read. - frankenphp.c: header included only when FRANKENPHP_TEST_HOOKS is defined. First real consumer (background workers) drops the guard. - Test hook gated on FRANKENPHP_TEST_HOOKS: - PHP function frankenphp_test_persist_roundtrip(mixed): mixed runs validate -> persist -> to_request -> free and returns the result. - Registered via zend_register_functions at MINIT so it never appears in ext_functions[] and never ships in production builds. - CI workflows set -DFRANKENPHP_TEST_HOOKS in CGO_CFLAGS (tests.yaml + sanitizers.yaml). windows.yaml is the release build, not a test runner, and stays untouched. ## Notes - Build verified both without the flag (production path, no unused-function warnings) and with it (test path). - The FRANKENPHP_TEST_HOOKS guard around the header include goes away in the PR that lands the first real caller; the test hook itself goes away in that same step once end-to-end tests cover the code paths.
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.
Second step of the split suggested in #2287: land the persistent-zval
subsystem as a standalone, reviewable header, independent of background
workers. This is the subsystem most likely to hide latent refcount or
memory-lifetime bugs; reviewing it in isolation is higher-signal than
finding issues inside a 3k-line diff.
What
persistent_zval.h(renamed from thebg_worker_vars.hdraft,prefix dropped for generality):
persistent_zval_validate— whitelist (scalars, arrays of allowedvalues, enum instances). Everything else fails fast.
persistent_zval_persist— deep-copy request → persistent (pemalloc)memory. Fast paths baked in: interned strings shared, opcache-
immutable arrays passed by pointer without copying or owning.
persistent_zval_free— deep-free; skips interned strings andimmutable arrays (borrowed, not owned).
persistent_zval_to_request— deep-copy persistent → fresh requestmemory. Enums re-resolved by class + case name on each read.
frankenphp.c: header included only whenFRANKENPHP_TEST_HOOKSisdefined. First real consumer (background workers) drops the guard.
Test hook gated on
FRANKENPHP_TEST_HOOKS:frankenphp_test_persist_roundtrip(mixed): mixedrunsvalidate → persist → to_request → free and returns the result.
zend_register_functionsat MINIT so it neverappears in
ext_functions[]and never ships in production builds.CI workflows set
-DFRANKENPHP_TEST_HOOKSinCGO_CFLAGS(
tests.yaml+sanitizers.yaml).windows.yamlis the releasebuild, not a test runner, and stays untouched.
Notes
unused-function warnings) and with it (test path).
FRANKENPHP_TEST_HOOKSguard around the header include goesaway in the PR that lands the first real caller; the test hook
itself goes away in that same step once end-to-end tests cover the
code paths.