From 14c41fd5f3fcbec868c915ae9cb144cd514a336a Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Mon, 31 Jul 2023 15:09:06 +0200 Subject: [PATCH] Remove opcache.consistency_checks --- UPGRADING | 5 +++++ ext/opcache/ZendAccelerator.c | 20 -------------------- ext/opcache/ZendAccelerator.h | 5 ----- ext/opcache/tests/gh8065.phpt | 26 -------------------------- ext/opcache/zend_accelerator_module.c | 15 --------------- ext/opcache/zend_file_cache.c | 7 +++---- php.ini-development | 4 ---- php.ini-production | 4 ---- 8 files changed, 8 insertions(+), 78 deletions(-) delete mode 100644 ext/opcache/tests/gh8065.phpt diff --git a/UPGRADING b/UPGRADING index 11668111f7fd7..6ff91d8a43a75 100644 --- a/UPGRADING +++ b/UPGRADING @@ -54,6 +54,11 @@ PHP 8.3 UPGRADE NOTES . C functions that have a return type of void now return null instead of returning the following object object(FFI\CData:void) { } +- Opcache: + . The opcache.consistency_checks INI directive was removed. It was previously + disabled automatically without an option to enable it, due to buggy + behavior. See https://github.com/php/php-src/issues/8065 for details. + - Standard: . The range() function has had various changes: * A TypeError is now thrown when passing objects, resources, or arrays diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 32cf51301329d..0346a44e3dc0b 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -1538,8 +1538,6 @@ static zend_persistent_script *store_script_in_file_cache(zend_persistent_script (size_t)ZCG(mem)); } - new_persistent_script->dynamic_members.checksum = zend_accel_script_checksum(new_persistent_script); - zend_file_cache_script_store(new_persistent_script, /* is_shm */ false); return new_persistent_script; @@ -1653,8 +1651,6 @@ static zend_persistent_script *cache_script_in_shared_memory(zend_persistent_scr (size_t)ZCG(mem)); } - new_persistent_script->dynamic_members.checksum = zend_accel_script_checksum(new_persistent_script); - /* store script structure in the hash table */ bucket = zend_accel_hash_update(&ZCSG(hash), new_persistent_script->script.filename, 0, new_persistent_script); if (bucket) { @@ -2131,20 +2127,6 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) } } - /* if turned on - check the compiled script ADLER32 checksum */ - if (persistent_script && ZCG(accel_directives).consistency_checks - && persistent_script->dynamic_members.hits % ZCG(accel_directives).consistency_checks == 0) { - - unsigned int checksum = zend_accel_script_checksum(persistent_script); - if (checksum != persistent_script->dynamic_members.checksum ) { - /* The checksum is wrong */ - zend_accel_error(ACCEL_LOG_INFO, "Checksum failed for '%s': expected=0x%08x, found=0x%08x", - ZSTR_VAL(persistent_script->script.filename), persistent_script->dynamic_members.checksum, checksum); - zend_accel_lock_discard_script(persistent_script); - persistent_script = NULL; - } - } - /* Check the second level cache */ if (!persistent_script && ZCG(accel_directives).file_cache) { persistent_script = zend_file_cache_script_load(file_handle); @@ -4280,8 +4262,6 @@ static zend_persistent_script* preload_script_in_shared_memory(zend_persistent_s (size_t)ZCG(mem)); } - new_persistent_script->dynamic_members.checksum = zend_accel_script_checksum(new_persistent_script); - /* store script structure in the hash table */ bucket = zend_accel_hash_update(&ZCSG(hash), new_persistent_script->script.filename, 0, new_persistent_script); if (bucket) { diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h index 568d8f4972419..1aa1655f43eb2 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -132,14 +132,10 @@ typedef struct _zend_persistent_script { void *mem; /* shared memory area used by script structures */ size_t size; /* size of used shared memory */ - /* All entries that shouldn't be counted in the ADLER32 - * checksum must be declared in this struct - */ struct zend_persistent_script_dynamic_members { time_t last_used; zend_ulong hits; unsigned int memory_consumption; - unsigned int checksum; time_t revalidate; } dynamic_members; } zend_persistent_script; @@ -149,7 +145,6 @@ typedef struct _zend_accel_directives { zend_long max_accelerated_files; double max_wasted_percentage; char *user_blacklist_filename; - zend_long consistency_checks; zend_long force_restart_timeout; bool use_cwd; bool ignore_dups; diff --git a/ext/opcache/tests/gh8065.phpt b/ext/opcache/tests/gh8065.phpt deleted file mode 100644 index db951a7b0917b..0000000000000 --- a/ext/opcache/tests/gh8065.phpt +++ /dev/null @@ -1,26 +0,0 @@ ---TEST-- -GH-8065: opcache.consistency_checks > 0 causes segfaults in PHP >= 8.1.5 in fpm context ---EXTENSIONS-- -opcache ---INI-- -opcache.enable_cli=1 -opcache.consistency_checks=1 -opcache.log_verbosity_level=2 ---FILE-- - ---EXPECTF-- -%sWarning opcache.consistency_checks is reset back to 0 because it does not work properly (see GH-8065, GH-10624). - -string(1) "0" -%sWarning opcache.consistency_checks is reset back to 0 because it does not work properly (see GH-8065, GH-10624). - -bool(false) -%sWarning opcache.consistency_checks is reset back to 0 because it does not work properly (see GH-8065, GH-10624). - -bool(false) -string(1) "0" diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 5181af8d8c995..dc0fa961d7446 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -129,19 +129,6 @@ static ZEND_INI_MH(OnUpdateMaxWastedPercentage) return SUCCESS; } -static ZEND_INI_MH(OnUpdateConsistencyChecks) -{ - zend_long *p = (zend_long *) ZEND_INI_GET_ADDR(); - zend_long consistency_checks = atoi(ZSTR_VAL(new_value)); - - if (consistency_checks != 0) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache.consistency_checks is reset back to 0 because it does not work properly (see GH-8065, GH-10624).\n"); - return FAILURE; - } - *p = 0; - return SUCCESS; -} - static ZEND_INI_MH(OnEnable) { if (stage == ZEND_INI_STAGE_STARTUP || @@ -289,7 +276,6 @@ ZEND_INI_BEGIN() STD_PHP_INI_ENTRY("opcache.interned_strings_buffer", "8" , PHP_INI_SYSTEM, OnUpdateInternedStringsBuffer, accel_directives.interned_strings_buffer, zend_accel_globals, accel_globals) STD_PHP_INI_ENTRY("opcache.max_accelerated_files" , "10000", PHP_INI_SYSTEM, OnUpdateMaxAcceleratedFiles, accel_directives.max_accelerated_files, zend_accel_globals, accel_globals) STD_PHP_INI_ENTRY("opcache.max_wasted_percentage" , "5" , PHP_INI_SYSTEM, OnUpdateMaxWastedPercentage, accel_directives.max_wasted_percentage, zend_accel_globals, accel_globals) - STD_PHP_INI_ENTRY("opcache.consistency_checks" , "0" , PHP_INI_ALL , OnUpdateConsistencyChecks, accel_directives.consistency_checks, zend_accel_globals, accel_globals) STD_PHP_INI_ENTRY("opcache.force_restart_timeout" , "180" , PHP_INI_SYSTEM, OnUpdateLong, accel_directives.force_restart_timeout, zend_accel_globals, accel_globals) STD_PHP_INI_ENTRY("opcache.revalidate_freq" , "2" , PHP_INI_ALL , OnUpdateLong, accel_directives.revalidate_freq, zend_accel_globals, accel_globals) STD_PHP_INI_ENTRY("opcache.file_update_protection", "2" , PHP_INI_ALL , OnUpdateLong, accel_directives.file_update_protection, zend_accel_globals, accel_globals) @@ -804,7 +790,6 @@ ZEND_FUNCTION(opcache_get_configuration) add_assoc_long(&directives, "opcache.interned_strings_buffer",ZCG(accel_directives).interned_strings_buffer); add_assoc_long(&directives, "opcache.max_accelerated_files", ZCG(accel_directives).max_accelerated_files); add_assoc_double(&directives, "opcache.max_wasted_percentage", ZCG(accel_directives).max_wasted_percentage); - add_assoc_long(&directives, "opcache.consistency_checks", ZCG(accel_directives).consistency_checks); add_assoc_long(&directives, "opcache.force_restart_timeout", ZCG(accel_directives).force_restart_timeout); add_assoc_long(&directives, "opcache.revalidate_freq", ZCG(accel_directives).revalidate_freq); add_assoc_string(&directives, "opcache.preferred_memory_model", STRING_NOT_NULL(ZCG(accel_directives).memory_model)); diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index 864bc4aff202c..db9fabfa4b50f 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -1118,9 +1118,6 @@ int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm) zend_string *const s = (zend_string*)ZCG(mem); - info.checksum = zend_adler32(ADLER32_INIT, buf, script->size); - info.checksum = zend_adler32(info.checksum, (unsigned char*)ZSTR_VAL(s), info.str_size); - #if __has_feature(memory_sanitizer) /* The buffer may contain uninitialized regions. However, the uninitialized parts will not be * used when reading the cache. We should probably still try to get things fully initialized @@ -1129,6 +1126,9 @@ int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm) __msan_unpoison(buf, script->size); #endif + info.checksum = zend_adler32(ADLER32_INIT, buf, script->size); + info.checksum = zend_adler32(info.checksum, (unsigned char*)ZSTR_VAL(s), info.str_size); + if (!zend_file_cache_script_write(fd, script, &info, buf, s)) { zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot write to file '%s': %s\n", filename, strerror(errno)); zend_string_release_ex(s, 0); @@ -1935,7 +1935,6 @@ zend_persistent_script *zend_file_cache_script_load(zend_file_handle *file_handl if (cache_it) { ZCSG(map_ptr_last) = CG(map_ptr_last); - script->dynamic_members.checksum = zend_accel_script_checksum(script); script->dynamic_members.last_used = ZCG(request_time); zend_accel_hash_update(&ZCSG(hash), script->script.filename, 0, script); diff --git a/php.ini-development b/php.ini-development index e3293e8ffa50d..c837e82c2fe8e 100644 --- a/php.ini-development +++ b/php.ini-development @@ -1841,10 +1841,6 @@ ldap.max_links = -1 ; are cached. ;opcache.max_file_size=0 -; Check the cache checksum each N requests. -; The default value of "0" means that the checks are disabled. -;opcache.consistency_checks=0 - ; How long to wait (in seconds) for a scheduled restart to begin if the cache ; is not being accessed. ;opcache.force_restart_timeout=180 diff --git a/php.ini-production b/php.ini-production index beebc0461b472..0c9c82f56a348 100644 --- a/php.ini-production +++ b/php.ini-production @@ -1843,10 +1843,6 @@ ldap.max_links = -1 ; are cached. ;opcache.max_file_size=0 -; Check the cache checksum each N requests. -; The default value of "0" means that the checks are disabled. -;opcache.consistency_checks=0 - ; How long to wait (in seconds) for a scheduled restart to begin if the cache ; is not being accessed. ;opcache.force_restart_timeout=180