Skip to content

Fix GH-23859: Reset map pointer watermark in accel_activate() - #24644

Open
aasul wants to merge 2 commits into
php:PHP-8.4from
aasul:fix/gh23859-opcache-reset-84
Open

aasul wants to merge 2 commits into
php:PHP-8.4from
aasul:fix/gh23859-opcache-reset-84

Conversation

@aasul

@aasul aasul commented Oct 24, 2026

Copy link
Copy Markdown

Fixes #23859

i looked into the map pointer growth after repeated opcache_reset() calls and found that surviving FPM workers could keep an outdated CG(map_ptr_last) value after a restart

when those workers cached new scripts the old watermark would get published back into shared memory causing the map pointer table to keep growing after each reset

the fix resets the local watermark during activation while keeping startup and preloaded slots intact

i also added an FPM regression test using two separate workers to make sure the map pointer count stays stable when another worker triggers a reset

the test fails on the original code and passes with the fix, i also ran it 20 times without any failures

after 24 reset cycles the original version went from 933 to over 18000 map pointer slots while the patched version stayed at 933

NTS and ZTS both passed 6031 tests with no failures and i also checked concurrent FPM requests, worker replacement, preloading, JIT and ASan

LeakSanitizer still reported some allocations during worker shutdown but the same ones also appeared on unchanged builds so nothing new seems to have been introduced

i haven't tested Windows, 32 bit platforms or threaded web SAPIs

investigation and testing were assisted by AI and fully reviewed by human eyes.

@aasul
aasul requested review from bukka and dstogov as code owners October 24, 2026 13:38
Copilot AI balanced review requested due to automatic review settings October 24, 2026 13:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The watermark reset may alias preloaded regular map-pointer slots, and the regression test needs static-OPcache compatibility and cleanup fixes.

1 open issue

Review effort: Lite

Comment thread ext/opcache/ZendAccelerator.c

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The change affects OPcache shared-memory state and worker activation across multiple runtime modes, so final human approval is warranted.

No open issues 🎉

1 resolved since last review

Review effort: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants