Skip to content

Fix GH-17733: System entropy does not take into account function order, resulting in wrong functions possibly called in different configurations - #24664

Closed
ndossche wants to merge 1 commit into
php:PHP-8.4from
ndossche:fix-17733
Closed

Fix GH-17733: System entropy does not take into account function order, resulting in wrong functions possibly called in different configurations#24664
ndossche wants to merge 1 commit into
php:PHP-8.4from
ndossche:fix-17733

Conversation

@ndossche

@ndossche ndossche commented Oct 24, 2026

Copy link
Copy Markdown
Member

This properly fixes the entropy computation to take into account function ordering and module versions. This is basically a reintroduction of the previously removed code from GH-5871, but a simpler version. Since this runs once every module startup, the overhead is negligible as it does not influence the request.

I chose this simple approach rather than the suggestion to introduce multiple system entropies based on shm/file-cache.

This PR also simplifies some other parts, since we can get rid of some workarounds.

@ndossche
Fix phpGH-17733: System entropy does not take into account function o…
18794dc 
…rder, resulting in wrong functions possibly called in different configurations

This properly fixes the entropy computation to take into account
function ordering and module versions. This is basically a
reintroduction of the previously removed code from phpGH-5871, but a
simpler version. Since this runs once every module startup, the overhead
is negligible as it does not influence the request.

I chose this simple approach rather than the suggestion to introduce
multiple system entropies based on shm/file-cache.

This PR also simplifies some other parts, since we can get rid of some
workarounds.
@ndossche
ndossche requested a review from dstogov as a code owner October 24, 2026 22:11
@ndossche ndossche linked an issue Oct 24, 2026 that may be closed by this pull request
System entropy does not take into account function order, resulting in wrong functions possibly called in different configurations #17733
Closed
@Enfiladeguide

Enfiladeguide commented Oct 25, 2026 •
edited
Loading

Copy link
Copy Markdown

Quoted text written with an LLM and checked by a human developer.

I ran this branch against the FPM setup from #24661. gh17733.phpt passes with the patch and fails without it, but the FPM case is unchanged: two pools under one master, pool a with php_admin_value[disable_functions] = exec, pool b without. t.php (echo var_export([1, 'x' => true], true), "\n";), requested through a and then through b:

pool a: array (   0 => 1,   'x' => true, )
pool b: array(2) {   [0]=>   int(1)   ["x"]=>   bool(true) } bool(true)

The output is the same on PHP-8.4 at 51808ac and on this branch at 18794dc (both built with --disable-all --enable-fpm --enable-opcache, opcache loaded with default settings; newlines shown as spaces).

As far as I can see, the system ID can't catch this case. It is finalized once in the master, in php_module_startup(), while a pool's disable_functions is applied later in each worker (fpm_php_init_child() → zend_disable_functions()). On Linux the shared-memory cache doesn't check the system ID at all.

@Enfiladeguide Enfiladeguide mentioned this pull request Oct 25, 2026
Open
arnaud-lb
arnaud-lb approved these changes Oct 26, 2026
View reviewed changes

@arnaud-lb arnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me!

@ndossche ndossche closed this in d6541df Oct 26, 2026
@Enfiladeguide Enfiladeguide mentioned this pull request Oct 26, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arnaud-lb arnaud-lb arnaud-lb approved these changes
@iluuu1994 iluuu1994 Awaiting requested review from iluuu1994
@dstogov dstogov Awaiting requested review from dstogov dstogov is a code owner

Assignees

No one assigned

Labels

Category: Engine Extension: opcache

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

System entropy does not take into account function order, resulting in wrong functions possibly called in different configurations

3 participants

@ndossche @Enfiladeguide @arnaud-lb