From 02f5c541e2177d1bd359f4612cf4feda253577ea Mon Sep 17 00:00:00 2001 From: KayanoLiam Date: Thu, 5 Nov 2026 16:29:45 +0800 Subject: [PATCH] Fix GH-24361: Restore GC after PHP opcache_compile_file errors Record fatal error origin before bailout and restore GC only for direct compiler failures, preserving runtime fatal protection Cover manual and automatic collection, error handlers and fatal callbacks --- NEWS | 2 + ext/opcache/ZendAccelerator.c | 2 + ext/opcache/tests/gh24361.inc | 2 + ext/opcache/tests/gh24361.phpt | 53 +++++++++++++++++++ ext/opcache/tests/gh24361_error_handler.phpt | 34 ++++++++++++ ext/opcache/tests/gh24361_memory_limit.phpt | 41 ++++++++++++++ ext/opcache/tests/gh24361_nested_fatal.phpt | 35 ++++++++++++ .../tests/gh24361_repeated_errors.phpt | 39 ++++++++++++++ ext/opcache/zend_accelerator_module.c | 25 +++++++++ ext/opcache/zend_accelerator_module.h | 3 ++ 10 files changed, 236 insertions(+) create mode 100644 ext/opcache/tests/gh24361.inc create mode 100644 ext/opcache/tests/gh24361.phpt create mode 100644 ext/opcache/tests/gh24361_error_handler.phpt create mode 100644 ext/opcache/tests/gh24361_memory_limit.phpt create mode 100644 ext/opcache/tests/gh24361_nested_fatal.phpt create mode 100644 ext/opcache/tests/gh24361_repeated_errors.phpt diff --git a/NEWS b/NEWS index f13aa1a98174..ea33053b3ab9 100644 --- a/NEWS +++ b/NEWS @@ -50,6 +50,8 @@ PHP NEWS . Fixed use-after-free in JIT when the error handler releases a string while an "Undefined variable" warning is raised for its offset in ?? or nested isset()/empty(). (Jorg Sowa) + . Fixed bug GH-24361 (opcache_compile_file() disables cycle collection after + a compile error). (akasakariko) - PDO_Firebird: . Fixed stack buffer overflow with long connection user names, passwords, diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index e56904d7efb8..49874dc451de 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -3241,6 +3241,8 @@ static int accel_startup(zend_extension *extension) /* Prevent unloading */ extension->handle = 0; + zend_observer_error_register(zend_accel_record_fatal_error); + return SUCCESS; } diff --git a/ext/opcache/tests/gh24361.inc b/ext/opcache/tests/gh24361.inc new file mode 100644 index 000000000000..e06e2f86e0bd --- /dev/null +++ b/ext/opcache/tests/gh24361.inc @@ -0,0 +1,2 @@ +self = $node; +unset($node); +var_dump(gc_collect_cycles()); + +for ($i = 0; $i < 100000; $i++) { + $node = new stdClass; + $node->self = $node; +} +unset($node); +var_dump(gc_status()['runs'] > 1); +gc_collect_cycles(); +var_dump(gc_status()['collected']); + +ini_set('opcache.enable', '0'); +gc_disable(); +var_dump(@opcache_compile_file(__DIR__ . '/gh24361.inc')); +var_dump(gc_status()['protected']); +var_dump(gc_enabled()); +$node = new stdClass; +$node->self = $node; +unset($node); +var_dump(gc_collect_cycles()); +?> +--EXPECTF-- +bool(false) + +Fatal error: 'break' not in the 'loop' or 'switch' context in %sgh24361.inc on line 2 +bool(false) +bool(false) +int(1) +bool(true) +int(100001) + +Fatal error: 'break' not in the 'loop' or 'switch' context in %sgh24361.inc on line 2 +bool(false) +bool(false) +bool(false) +int(1) diff --git a/ext/opcache/tests/gh24361_error_handler.phpt b/ext/opcache/tests/gh24361_error_handler.phpt new file mode 100644 index 000000000000..773d16d4bf23 --- /dev/null +++ b/ext/opcache/tests/gh24361_error_handler.phpt @@ -0,0 +1,34 @@ +--TEST-- +GH-24361 (GC is restored before reporting the opcache_compile_file() warning) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.file_update_protection=0 +--FILE-- +self = $node; + unset($node); + var_dump(gc_collect_cycles()); + throw new RuntimeException('compile warning'); +}); + +try { + opcache_compile_file(__DIR__ . '/gh24361.inc'); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), "\n"; +} +var_dump(gc_status()['protected']); +?> +--EXPECTF-- +Fatal error: 'break' not in the 'loop' or 'switch' context in %sgh24361.inc on line 2 +bool(true) +bool(false) +int(1) +RuntimeException: compile warning +bool(false) diff --git a/ext/opcache/tests/gh24361_memory_limit.phpt b/ext/opcache/tests/gh24361_memory_limit.phpt new file mode 100644 index 000000000000..5bd3cba2e303 --- /dev/null +++ b/ext/opcache/tests/gh24361_memory_limit.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-24361 (memory exhaustion while opening a script keeps GC protected) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.file_update_protection=0 +memory_limit=16M +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +bool(false) + +Fatal error: Allowed memory size of %d bytes exhausted %s in %s on line %d +bool(true) diff --git a/ext/opcache/tests/gh24361_nested_fatal.phpt b/ext/opcache/tests/gh24361_nested_fatal.phpt new file mode 100644 index 000000000000..bff22eab7485 --- /dev/null +++ b/ext/opcache/tests/gh24361_nested_fatal.phpt @@ -0,0 +1,35 @@ +--TEST-- +GH-24361 (bailouts from user code and existing GC protection are preserved) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.file_update_protection=0 +--FILE-- + +--EXPECTF-- +bool(false) + +Fatal error: 'break' not in the 'loop' or 'switch' context in %s on line 1 +bool(false) +bool(true) + +Fatal error: 'break' not in the 'loop' or 'switch' context in %sgh24361.inc on line 2 +bool(false) +bool(true) diff --git a/ext/opcache/tests/gh24361_repeated_errors.phpt b/ext/opcache/tests/gh24361_repeated_errors.phpt new file mode 100644 index 000000000000..e8433d2f568e --- /dev/null +++ b/ext/opcache/tests/gh24361_repeated_errors.phpt @@ -0,0 +1,39 @@ +--TEST-- +GH-24361 (GC recovery does not rely on the last reported error) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.file_update_protection=0 +ignore_repeated_errors=1 +ignore_repeated_source=1 +display_errors=0 +log_errors=0 +--FILE-- + true, E_WARNING | E_DEPRECATED); +for ($i = 0; $i < 2; $i++) { + var_dump(opcache_compile_file(__DIR__ . '/gh24361.inc')); + var_dump(gc_status()['protected']); +} +var_dump(opcache_compile_file('repeated-error://test')); +var_dump(gc_status()['protected']); +var_dump(error_get_last()['type'] === E_COMPILE_ERROR); +?> +--EXPECT-- +bool(false) +bool(false) +bool(false) +bool(false) +bool(false) +bool(true) +bool(true) diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 902d69374bb7..e796a113c064 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -27,6 +27,7 @@ #include "zend_closures.h" #include "zend_shared_alloc.h" #include "zend_accelerator_blacklist.h" +#include "zend_accelerator_module.h" #include "php_ini.h" #include "SAPI.h" #include "zend_virtual_cwd.h" @@ -59,6 +60,22 @@ static zif_handler orig_file_exists = NULL; static zif_handler orig_is_file = NULL; static zif_handler orig_is_readable = NULL; +ZEND_TLS int accel_last_fatal_error_type; +ZEND_TLS zend_execute_data *accel_last_fatal_error_execute_data; +ZEND_TLS bool accel_last_fatal_error_gc_protected; + +void zend_accel_record_fatal_error( + int type, zend_string *error_filename, uint32_t error_lineno, zend_string *message) +{ + if (type & E_FATAL_ERRORS) { + /* Record the origin before zend_bailout() clears the current execution frame. + * Unlike last_error_type, notifications also cover ignored repeated errors. */ + accel_last_fatal_error_type = type; + accel_last_fatal_error_execute_data = EG(current_execute_data); + accel_last_fatal_error_gc_protected = gc_protected(); + } +} + static int validate_api_restriction(void) { if (ZCG(accel_directives).restrict_api && *ZCG(accel_directives).restrict_api) { @@ -985,10 +1002,18 @@ ZEND_FUNCTION(opcache_compile_file) * state. */ op_array = persistent_compile_file(&handle, ZEND_INCLUDE); } else { + accel_last_fatal_error_type = 0; zend_try { op_array = persistent_compile_file(&handle, ZEND_INCLUDE); } zend_catch { EG(current_execute_data) = orig_execute_data; + /* Only recover GC after a compile error in this frame, not a fatal error + * that unwound user code called by a stream wrapper or another hook. */ + if (accel_last_fatal_error_type == E_COMPILE_ERROR + && accel_last_fatal_error_execute_data == orig_execute_data + && !ZEND_HAS_OOM()) { + gc_protect(accel_last_fatal_error_gc_protected); + } zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " could not compile file %s", ZSTR_VAL(handle.filename)); } zend_end_try(); } diff --git a/ext/opcache/zend_accelerator_module.h b/ext/opcache/zend_accelerator_module.h index 656336eeba76..cd2f2f06674d 100644 --- a/ext/opcache/zend_accelerator_module.h +++ b/ext/opcache/zend_accelerator_module.h @@ -26,4 +26,7 @@ int start_accel_module(void); void zend_accel_override_file_functions(void); +void zend_accel_record_fatal_error( + int type, zend_string *error_filename, uint32_t error_lineno, zend_string *message); + #endif /* _ZEND_ACCELERATOR_MODULE_H */