Skip to content

feat: persistent-zval helpers (deep-copy zval trees across threads)#2366

Open
nicolas-grekas wants to merge 1 commit intophp:mainfrom
nicolas-grekas:persistent-zval-helpers
Open

feat: persistent-zval helpers (deep-copy zval trees across threads)#2366
nicolas-grekas wants to merge 1 commit intophp:mainfrom
nicolas-grekas:persistent-zval-helpers

Conversation

@nicolas-grekas
Copy link
Copy Markdown
Contributor

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 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.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant