From fb6a7d88a5d5304b70d3b512ccf96101ad4551db Mon Sep 17 00:00:00 2001 From: Alessio Attilio Date: Sat, 24 Oct 2026 15:52:43 +0200 Subject: [PATCH] Fix GH-23586: Keep GC traversal consistent under memory exhaustion --- NEWS | 2 ++ Zend/tests/gh23586.phpt | 42 ++++++++++++++++++++++++++++++++ Zend/tests/gh23586_002.phpt | 23 ++++++++++++++++++ Zend/tests/gh23586_003.phpt | 30 +++++++++++++++++++++++ Zend/zend_gc.c | 48 ++++++++++++++++++++++++++----------- 5 files changed, 131 insertions(+), 14 deletions(-) create mode 100644 Zend/tests/gh23586.phpt create mode 100644 Zend/tests/gh23586_002.phpt create mode 100644 Zend/tests/gh23586_003.phpt diff --git a/NEWS b/NEWS index b52fdfe6dc81..bc65161789c5 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,8 @@ PHP NEWS ?? ??? ????, PHP 8.4.28 - Core: + . Fixed bug GH-23586 (Assertion failure during garbage collection upon memory + exhaustion). (Alessio Attilio) . Fixed bug GH-24184 (System ID not being reinitialized after an Apache graceful restart, potentially causing incompatible OPcache file cache entries to be reused). (Levi Morrison) diff --git a/Zend/tests/gh23586.phpt b/Zend/tests/gh23586.phpt new file mode 100644 index 000000000000..3ff7066a7d6b --- /dev/null +++ b/Zend/tests/gh23586.phpt @@ -0,0 +1,42 @@ +--TEST-- +GH-23586 (GC traversal stack allocation must not exhaust the memory limit) +--SKIPIF-- + +--INI-- +memory_limit=93M +--FILE-- +children[] = $this; + } + $this->children[] = $this; + } + + public function __destruct() { + $this->children = null; + } +} + +for ($n = 0; $n < 2; $n++) { + $top = new Node(); + for ($i = 0; $i < 16; $i++) { + $ci = new Node($top); + for ($j = 0; $j < 16; $j++) { + $cj = new Node($ci); + while ($n) { + $ck = new Node($cj); + } + } + } +} +?> +--EXPECTF-- +Fatal error: Allowed memory size of %d bytes exhausted%S (tried to allocate %d bytes) in %s on line %d diff --git a/Zend/tests/gh23586_002.phpt b/Zend/tests/gh23586_002.phpt new file mode 100644 index 000000000000..3216eeca0afa --- /dev/null +++ b/Zend/tests/gh23586_002.phpt @@ -0,0 +1,23 @@ +--TEST-- +GH-23586 (GC can finish a wide traversal at the memory limit) +--SKIPIF-- + +--INI-- +memory_limit=256M +--FILE-- +children[] = (object) ['parent' => $root]; +} +unset($root); +ini_set('memory_limit', (string) memory_get_usage(true)); +var_dump(gc_collect_cycles()); +?> +--EXPECT-- +int(300002) diff --git a/Zend/tests/gh23586_003.phpt b/Zend/tests/gh23586_003.phpt new file mode 100644 index 000000000000..106df58342f7 --- /dev/null +++ b/Zend/tests/gh23586_003.phpt @@ -0,0 +1,30 @@ +--TEST-- +GH-23586 (GC traversal storage is released when a destructor bails out) +--SKIPIF-- + +--INI-- +error_reporting=E_ALL & ~E_DEPRECATED +--FILE-- +children[] = (object) ['parent' => $root]; +} +unset($root); +gc_collect_cycles(); +?> +--EXPECTF-- +Fatal error: Destructor bailout in %s on line %d diff --git a/Zend/zend_gc.c b/Zend/zend_gc.c index eb54ead7b110..5e23fc0bd225 100644 --- a/Zend/zend_gc.c +++ b/Zend/zend_gc.c @@ -333,7 +333,8 @@ struct _gc_stack { static zend_never_inline gc_stack* gc_stack_next(gc_stack *stack) { if (UNEXPECTED(!stack->next)) { - gc_stack *segment = emalloc(sizeof(gc_stack)); + /* Traversal must not bail out on the request memory limit after changing refcounts. */ + gc_stack *segment = pemalloc(sizeof(gc_stack), true); segment->prev = stack; segment->next = NULL; stack->next = segment; @@ -371,7 +372,7 @@ static void gc_stack_free(gc_stack *stack) while (p) { stack = p->next; - efree(p); + pefree(p, true); p = stack; } } @@ -1943,7 +1944,7 @@ static zend_never_inline void gc_call_destructors_in_fiber(uint32_t end) } } -ZEND_API int zend_gc_collect_cycles(void) +static int zend_gc_collect_cycles_impl(gc_stack *stack) { int total_count = 0; bool should_rerun_gc = 0; @@ -1961,10 +1962,6 @@ ZEND_API int zend_gc_collect_cycles(void) zend_refcounted *p; uint32_t gc_flags = 0; uint32_t idx, end; - gc_stack stack; - - stack.prev = NULL; - stack.next = NULL; if (GC_G(gc_active)) { GC_G(collector_time) += zend_hrtime() - start_time; @@ -1976,17 +1973,16 @@ ZEND_API int zend_gc_collect_cycles(void) GC_G(gc_active) = 1; GC_TRACE("Marking roots"); - gc_mark_roots(&stack); + gc_mark_roots(stack); GC_TRACE("Scanning roots"); - gc_scan_roots(&stack); + gc_scan_roots(stack); GC_TRACE("Collecting roots"); - count = gc_collect_roots(&gc_flags, &stack); + count = gc_collect_roots(&gc_flags, stack); if (!GC_G(num_roots)) { /* nothing to free */ GC_TRACE("Nothing to free"); - gc_stack_free(&stack); GC_G(gc_active) = 0; goto finish; } @@ -2035,7 +2031,7 @@ ZEND_API int zend_gc_collect_cycles(void) while (idx != end) { if (GC_IS_DTOR_GARBAGE(current->ref)) { p = GC_GET_PTR(current->ref); - count -= gc_remove_nested_data_from_buffer(p, current, &stack); + count -= gc_remove_nested_data_from_buffer(p, current, stack); } current++; idx++; @@ -2058,8 +2054,6 @@ ZEND_API int zend_gc_collect_cycles(void) } } - gc_stack_free(&stack); - /* Destroy zvals. The root buffer may be reallocated. */ GC_TRACE("Destroying zvals"); zend_hrtime_t free_start_time = zend_hrtime(); @@ -2144,6 +2138,32 @@ ZEND_API int zend_gc_collect_cycles(void) return total_count; } +static int zend_gc_collect_cycles_with_stack(gc_stack *stack) +{ + int count; + + zend_try { + count = zend_gc_collect_cycles_impl(stack); + } zend_catch { + gc_stack_free(stack); + zend_bailout(); + } zend_end_try(); + + gc_stack_free(stack); + return count; +} + +ZEND_API int zend_gc_collect_cycles(void) +{ + /* Keep the modified stack outside the function containing setjmp(), so it remains + * valid when freeing traversal storage after a bailout. */ + gc_stack stack; + + stack.prev = NULL; + stack.next = NULL; + return zend_gc_collect_cycles_with_stack(&stack); +} + ZEND_API void zend_gc_get_status(zend_gc_status *status) { status->active = GC_G(gc_active);