Skip to content

Improve opcache.max_accelerated_files validation - #22412

Merged
jorgsowa merged 2 commits into
php:masterfrom
jack-worman:Better-opcache.max_accelerated_files-validation
Nov 3, 2026
Merged

Improve opcache.max_accelerated_files validation#22412
jorgsowa merged 2 commits into
php:masterfrom
jack-worman:Better-opcache.max_accelerated_files-validation

Conversation

@jack-worman

@jack-worman jack-worman commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

I think it improves clarity to be explicit about what values are allowed and to not implicitly coerce them.

At a minimum, MAX_ACCEL_FILES should be increased to be the real max value as this is a footgun.

@jack-worman
jack-worman requested a review from dstogov as a code owner June 23, 2026 13:22
@jack-worman
jack-worman marked this pull request as draft June 23, 2026 13:23
@jack-worman jack-worman changed the title Better-opcache.max_accelerated_files-validation Improve opcache.max_accelerated_files validation Jun 23, 2026
@TimWolla

TimWolla commented Jun 23, 2026

Copy link
Copy Markdown
Member

This change is acceptable for master only. Can you first retarget the PR and afterwards rebase your branch onto master?

@jack-worman
jack-worman force-pushed the Better-opcache.max_accelerated_files-validation branch from b342d67 to a276d2b Compare June 23, 2026 14:23
@jack-worman
jack-worman changed the base branch from PHP-8.3 to master June 23, 2026 14:23
@jack-worman
jack-worman marked this pull request as ready for review June 23, 2026 14:25
@jack-worman
jack-worman requested a review from TimWolla as a code owner June 23, 2026 14:25
@jack-worman

jack-worman commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

This change is acceptable for master only. Can you first retarget the PR and afterwards rebase your branch onto master?

Done.

TimWolla
TimWolla approved these changes Jun 23, 2026
View reviewed changes

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

This seems reasonable.

mvorisek
mvorisek reviewed Jun 23, 2026
View reviewed changes
Comment thread ext/opcache/zend_accelerator_module.c
&& size != 130987
&& size != 262237
&& size != 524521
&& size != 1048793

@mvorisek mvorisek Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the reason to not allow number larger than 1048793?

@jack-worman jack-worman Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure; I only know that 1048793 has been the max since 2013

@jack-worman jack-worman Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

and 1048793 is the largest entry in the prime_numbers array in ext/opcache/zend_accelerator_hash.c

@jack-worman

jack-worman commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@TimWolla @dstogov @mvorisek ping

@jack-worman
jack-worman force-pushed the Better-opcache.max_accelerated_files-validation branch from a276d2b to a5bab16 Compare August 4, 2026 18:02
@jack-worman
jack-worman force-pushed the Better-opcache.max_accelerated_files-validation branch from a5bab16 to 531bd54 Compare October 24, 2026 03:17
@jack-worman
jack-worman requested a review from mvorisek October 24, 2026 12:08
jorgsowa
jorgsowa approved these changes Nov 3, 2026
View reviewed changes

@jorgsowa jorgsowa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This makes sense. Can you add entry in UPGRADING?

@jack-worman
jack-worman force-pushed the Better-opcache.max_accelerated_files-validation branch from 531bd54 to 8741ae0 Compare November 3, 2026 14:26
@jack-worman

jack-worman commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

This makes sense. Can you add entry in UPGRADING?

done

@jack-worman

jack-worman commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Should this still target master or can we get it in PHP 8.6?
The branch for 8.6 did not exist when I made it.

@jorgsowa

jorgsowa commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

This should target master. Let's wait for whole CI to pass.

@jorgsowa

jorgsowa commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Thanks!

@jorgsowa
jorgsowa merged commit 51d3a88 into php:master Nov 3, 2026
18 checks passed
@jack-worman
jack-worman deleted the Better-opcache.max_accelerated_files-validation branch November 3, 2026 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TimWolla TimWolla TimWolla approved these changes
@jorgsowa jorgsowa jorgsowa approved these changes
@dstogov dstogov Awaiting requested review from dstogov dstogov is a code owner
@LamentXU123 LamentXU123 Awaiting requested review from LamentXU123 LamentXU123 is a code owner
@mvorisek mvorisek Awaiting requested review from mvorisek

Assignees

No one assigned

Labels

Category: CI Extension: opcache Extension: phar

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jack-worman @TimWolla @jorgsowa @mvorisek