fix(zai-config): dup INI values per ZTS thread instead of refcounting#3832
fix(zai-config): dup INI values per ZTS thread instead of refcounting#3832morrisonlevi merged 3 commits intomasterfrom
Conversation
…aring zai_config_ini_rinit was replacing each ZTS thread's per-thread zend_string (originally created by PHP's zend_copy_ini_directives via zend_string_dup) with zend_string_copy(source->value) — a refcount-shared pointer to the process-wide master in zai_config_memoized_entries[i].ini_entries[n]->value. PHP's refcount ops are not atomic. With many ZTS workers each running this rinit on every request, concurrent ++/-- on the shared refcount produced torn values, premature frees, and use-after-free reads / Zend MM heap corruption (observed as "zend_mm_heap corrupted" inside zend_string_release during this very function on FrankenPHP under heavy load). Match PHP's own thread-setup pattern: zend_string_dup gives each thread an independent persistent allocation with refcount 1 that is never raced. In the orig_value == ini->value aliased case, repoint ini->value to its own fresh dup before releasing orig_value to avoid a dangling pointer. Memory cost is ~tens of bytes per ZAI entry per thread; rinit churn is balanced release+dup, no growth. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: e60de64 | Docs | Datadog PR Page | Give us feedback! |
Benchmarks [ tracer ]Benchmark execution time: 2026-04-27 15:42:52 Comparing candidate commit e60de64 in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 193 metrics, 0 unstable metrics. scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache
|
|
Alternatively, we might just intern the persistent zai config ini values ( |
9047aa2 to
9d3d627
Compare
9d3d627 to
940690f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e60de64d6c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (identical_orig) { | ||
| ini->value = ini->orig_value; | ||
|
|
||
| if (!applied_update) { | ||
| if (ZaiConfigOnUpdateIni(ini, ini->value, NULL, NULL, NULL, PHP_INI_STAGE_RUNTIME) == SUCCESS) { |
There was a problem hiding this comment.
Replay modified INI values when orig and value differ
On ZTS, this change now only calls ZaiConfigOnUpdateIni(...) inside if (identical_orig), so entries with ini->modified == true and ini->orig_value != ini->value no longer reapply their runtime INI override during zai_config_ini_rinit. That is a regression because zai_config_runtime_config_ctor() repopulates request config from globals, and earlier INI on_modify callbacks are ignored when runtime config is not initialized yet (ZaiConfigOnUpdateIni returns early). In requests that rely on user.ini/htaccess/FPM runtime INI overrides, config values can silently fall back to global defaults.
Useful? React with 👍 / 👎.
SCP-1166
Description
Relates to #3729.
In ZTS mode,
zai_config_ini_rinitwas assigningzend_string_copy(source->value)to each thread's per-thread INI entry. This was a refcount-bump of the process-wide master zend_string stored inzai_config_memoized_entries. PHP's refcount operations are not atomic. With many ZTS workers each running rinit concurrently, the competing ++/-- on the shared refcount produced torn values, premature frees, and use-after-free reads, observed aszend_mm_heapcorrupted or crashes under heavy FrankenPHP load, particularly on aarch64.Reviewer checklist