From 115cd6f088af98ac91630d559d6f8426dc6d8a94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Thu, 28 Aug 2025 20:56:35 +0200 Subject: [PATCH 1/4] =?UTF-8?q?opcache:=20Do=20not=20emit=20=E2=80=9Ctempo?= =?UTF-8?q?rary=20enabling=E2=80=9D=20message=20when=20OPcache=20is=20alre?= =?UTF-8?q?ady=20active?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An easy way to accidentally enable OPcache “temporarily” is by using `php_admin_value[opcache.enable]=1` within a FPM pool’s configuration, since the `php_admin_value` settings mostly behave like settings in php.ini, with many OPcache INI settings being a notable exception. As long as OPcache is already enabled within php.ini (or simply by default), emitting a warning for `php_admin_value[opcache.enable]=1` or similar is going to be confusing, since is not actually temporarily enabling anything. A follow-up commit will also try to detect this kind of incorrect configuration and try to provide better advice for cases where OPcache is actually not yet enabled. --- .../tests/opcache_enable_noop_001.phpt | 24 +++++++++++++++++++ .../tests/opcache_enable_noop_002.phpt | 18 ++++++++++++++ ext/opcache/zend_accelerator_module.c | 5 ++++ 3 files changed, 47 insertions(+) create mode 100644 ext/opcache/tests/opcache_enable_noop_001.phpt create mode 100644 ext/opcache/tests/opcache_enable_noop_002.phpt diff --git a/ext/opcache/tests/opcache_enable_noop_001.phpt b/ext/opcache/tests/opcache_enable_noop_001.phpt new file mode 100644 index 0000000000000..c1fc826aa90e0 --- /dev/null +++ b/ext/opcache/tests/opcache_enable_noop_001.phpt @@ -0,0 +1,24 @@ +--TEST-- +Dynamically setting opcache.enable does not warn when noop +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +--EXTENSIONS-- +opcache +--FILE-- + +--EXPECTF-- +Should not warn: +Disabling: +Should warn: + +Warning: Zend OPcache can't be temporary enabled (it may be only disabled till the end of request) in %s on line %d diff --git a/ext/opcache/tests/opcache_enable_noop_002.phpt b/ext/opcache/tests/opcache_enable_noop_002.phpt new file mode 100644 index 0000000000000..0271aa75fcdc7 --- /dev/null +++ b/ext/opcache/tests/opcache_enable_noop_002.phpt @@ -0,0 +1,18 @@ +--TEST-- +Dynamically setting opcache.enable warns when not a noop +--INI-- +opcache.enable=0 +opcache.enable_cli=1 +--EXTENSIONS-- +opcache +--FILE-- + +--EXPECTF-- +Should warn, since the INI was initialized to 0: + +Warning: Zend OPcache can't be temporary enabled (it may be only disabled till the end of request) in %s on line %d diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 9a168b38baaf8..7d719f0c04806 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -162,6 +162,11 @@ static ZEND_INI_MH(OnEnable) /* It may be only temporary disabled */ bool *p = (bool *) ZEND_INI_GET_ADDR(); if (zend_ini_parse_bool(new_value)) { + if (*p) { + /* Do not warn if OPcache is enabled, as the update would be a noop anyways. */ + return SUCCESS; + } + zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporary enabled (it may be only disabled till the end of request)"); return FAILURE; } else { From fb1f245f6a6737ff1e93b8f891b2cf683651cfc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Thu, 28 Aug 2025 21:31:44 +0200 Subject: [PATCH 2/4] opcache: Improve error message when OPcache is enabled dynamically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The error message will now advice on the `php_admin_value[opcache.enable]=1` mistake. It will also send the message to OPcache’s logging facility instead of the regular error handling logic during startup so that it will not be made available to `error_get_last()`, since it is related to a specific request and thus not actionable by a script either. php/php-src#19146 made a related change to `opcache.memory_consumption`. --- ext/opcache/zend_accelerator_module.c | 10 +++- .../fpm/tests/opcache_enable_admin_value.phpt | 49 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 sapi/fpm/tests/opcache_enable_admin_value.phpt diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 7d719f0c04806..e296db1f5238e 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -167,7 +167,15 @@ static ZEND_INI_MH(OnEnable) return SUCCESS; } - zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporary enabled (it may be only disabled till the end of request)"); + if (stage == ZEND_INI_STAGE_ACTIVATE) { + if (strcmp(sapi_module.name, "fpm-fcgi") == 0) { + zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporary enabled. Are you using php_admin_value[opcache.enable]=1 in an individual pool's configuration?"); + } else { + zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporary enabled (it may be only disabled till the end of request)"); + } + } else { + zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporary enabled (it may be only disabled till the end of request)"); + } return FAILURE; } else { *p = 0; diff --git a/sapi/fpm/tests/opcache_enable_admin_value.phpt b/sapi/fpm/tests/opcache_enable_admin_value.phpt new file mode 100644 index 0000000000000..232b75b87bb4a --- /dev/null +++ b/sapi/fpm/tests/opcache_enable_admin_value.phpt @@ -0,0 +1,49 @@ +--TEST-- +Setting opcache.enable via php_admin_value fails gracefully +--SKIPIF-- + +--FILE-- +start(iniEntries: [ + 'opcache.enable' => '0', + 'opcache.log_verbosity_level' => '2', +]); +$tester->expectLogStartNotices(); +$tester->expectLogPattern("/Zend OPcache can't be temporary enabled. Are you using php_admin_value\\[opcache.enable\\]=1 in an individual pool's configuration?/"); +echo $tester + ->request() + ->getBody(); +$tester->terminate(); +$tester->close(); + +?> +--EXPECT-- +NULL +--CLEAN-- + From a533f7e7e519bfcc5a58cb8e0b0f76908c42cda9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Fri, 29 Aug 2025 15:02:42 +0200 Subject: [PATCH 3/4] opcache: Fix typo in warning message --- ext/opcache/tests/opcache_enable_noop_001.phpt | 2 +- ext/opcache/tests/opcache_enable_noop_002.phpt | 2 +- ext/opcache/zend_accelerator_module.c | 8 ++++---- sapi/fpm/tests/opcache_enable_admin_value.phpt | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ext/opcache/tests/opcache_enable_noop_001.phpt b/ext/opcache/tests/opcache_enable_noop_001.phpt index c1fc826aa90e0..1030a12a03d2f 100644 --- a/ext/opcache/tests/opcache_enable_noop_001.phpt +++ b/ext/opcache/tests/opcache_enable_noop_001.phpt @@ -21,4 +21,4 @@ Should not warn: Disabling: Should warn: -Warning: Zend OPcache can't be temporary enabled (it may be only disabled till the end of request) in %s on line %d +Warning: Zend OPcache can't be temporarily enabled (it may be only disabled till the end of request) in %s on line %d diff --git a/ext/opcache/tests/opcache_enable_noop_002.phpt b/ext/opcache/tests/opcache_enable_noop_002.phpt index 0271aa75fcdc7..bbddef9abadf2 100644 --- a/ext/opcache/tests/opcache_enable_noop_002.phpt +++ b/ext/opcache/tests/opcache_enable_noop_002.phpt @@ -15,4 +15,4 @@ ini_set('opcache.enable', 1); --EXPECTF-- Should warn, since the INI was initialized to 0: -Warning: Zend OPcache can't be temporary enabled (it may be only disabled till the end of request) in %s on line %d +Warning: Zend OPcache can't be temporarily enabled (it may be only disabled till the end of request) in %s on line %d diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index e296db1f5238e..88eb17606bc1e 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -159,7 +159,7 @@ static ZEND_INI_MH(OnEnable) stage == ZEND_INI_STAGE_DEACTIVATE) { return OnUpdateBool(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage); } else { - /* It may be only temporary disabled */ + /* It may be only temporarily disabled */ bool *p = (bool *) ZEND_INI_GET_ADDR(); if (zend_ini_parse_bool(new_value)) { if (*p) { @@ -169,12 +169,12 @@ static ZEND_INI_MH(OnEnable) if (stage == ZEND_INI_STAGE_ACTIVATE) { if (strcmp(sapi_module.name, "fpm-fcgi") == 0) { - zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporary enabled. Are you using php_admin_value[opcache.enable]=1 in an individual pool's configuration?"); + zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled. Are you using php_admin_value[opcache.enable]=1 in an individual pool's configuration?"); } else { - zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporary enabled (it may be only disabled till the end of request)"); + zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled (it may be only disabled till the end of request)"); } } else { - zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporary enabled (it may be only disabled till the end of request)"); + zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled (it may be only disabled till the end of request)"); } return FAILURE; } else { diff --git a/sapi/fpm/tests/opcache_enable_admin_value.phpt b/sapi/fpm/tests/opcache_enable_admin_value.phpt index 232b75b87bb4a..2c1081aed9eb8 100644 --- a/sapi/fpm/tests/opcache_enable_admin_value.phpt +++ b/sapi/fpm/tests/opcache_enable_admin_value.phpt @@ -32,7 +32,7 @@ $tester->start(iniEntries: [ 'opcache.log_verbosity_level' => '2', ]); $tester->expectLogStartNotices(); -$tester->expectLogPattern("/Zend OPcache can't be temporary enabled. Are you using php_admin_value\\[opcache.enable\\]=1 in an individual pool's configuration?/"); +$tester->expectLogPattern("/Zend OPcache can't be temporarily enabled. Are you using php_admin_value\\[opcache.enable\\]=1 in an individual pool's configuration?/"); echo $tester ->request() ->getBody(); From 0fc9bbf7fc6096f8782797f14a59935f1d5fba31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 30 Aug 2025 22:09:30 +0200 Subject: [PATCH 4/4] opcache: Use more formal language in warning message --- ext/opcache/tests/opcache_enable_noop_001.phpt | 2 +- ext/opcache/tests/opcache_enable_noop_002.phpt | 2 +- ext/opcache/zend_accelerator_module.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/opcache/tests/opcache_enable_noop_001.phpt b/ext/opcache/tests/opcache_enable_noop_001.phpt index 1030a12a03d2f..0e7f5bbc98793 100644 --- a/ext/opcache/tests/opcache_enable_noop_001.phpt +++ b/ext/opcache/tests/opcache_enable_noop_001.phpt @@ -21,4 +21,4 @@ Should not warn: Disabling: Should warn: -Warning: Zend OPcache can't be temporarily enabled (it may be only disabled till the end of request) in %s on line %d +Warning: Zend OPcache can't be temporarily enabled (it may be only disabled until the end of request) in %s on line %d diff --git a/ext/opcache/tests/opcache_enable_noop_002.phpt b/ext/opcache/tests/opcache_enable_noop_002.phpt index bbddef9abadf2..994f2d22f4e45 100644 --- a/ext/opcache/tests/opcache_enable_noop_002.phpt +++ b/ext/opcache/tests/opcache_enable_noop_002.phpt @@ -15,4 +15,4 @@ ini_set('opcache.enable', 1); --EXPECTF-- Should warn, since the INI was initialized to 0: -Warning: Zend OPcache can't be temporarily enabled (it may be only disabled till the end of request) in %s on line %d +Warning: Zend OPcache can't be temporarily enabled (it may be only disabled until the end of request) in %s on line %d diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 88eb17606bc1e..6f668af9b714d 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -171,10 +171,10 @@ static ZEND_INI_MH(OnEnable) if (strcmp(sapi_module.name, "fpm-fcgi") == 0) { zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled. Are you using php_admin_value[opcache.enable]=1 in an individual pool's configuration?"); } else { - zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled (it may be only disabled till the end of request)"); + zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled (it may be only disabled until the end of request)"); } } else { - zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled (it may be only disabled till the end of request)"); + zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled (it may be only disabled until the end of request)"); } return FAILURE; } else {