From edfe692ec72ef28e6fc6007910ae222c4fe183d4 Mon Sep 17 00:00:00 2001 From: lacatoire Date: Mon, 19 Oct 2026 13:45:24 +0200 Subject: [PATCH 1/3] Fix fiber switch corrupting user comparator globals in sort functions usort(), uasort(), uksort(), array_multisort() and the user-callback array_diff()/array_intersect() variants save and restore a request-global comparator in LIFO order. A comparator that suspends a Fiber lets another fiber's sort overwrite it, so the restore writes back a stale pointer (wrong comparator, or a segfault in array_multisort()). Block fiber switching while the global is live. Suspending from the comparator now throws a FiberError. --- ext/standard/array.c | 6 + .../array_sort_comparator_fiber_switch.phpt | 125 ++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 ext/standard/tests/array/array_sort_comparator_fiber_switch.phpt diff --git a/ext/standard/array.c b/ext/standard/array.c index 0b184efc3998..04790ffe4272 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -35,6 +35,7 @@ #include "zend_exceptions.h" #include "ext/random/php_random.h" #include "zend_frameless_function.h" +#include "zend_fibers.h" /* {{{ defines */ @@ -807,8 +808,11 @@ static int php_array_user_compare(Bucket *a, Bucket *b) /* {{{ */ old_user_compare_fci_cache = BG(user_compare_fci_cache); \ ARRAYG(compare_deprecation_thrown) = 0; \ BG(user_compare_fci_cache) = empty_fcall_info_cache; \ + /* The comparator is restored in LIFO order, which a fiber switch breaks. */ \ + zend_fiber_switch_block(); \ #define PHP_ARRAY_CMP_FUNC_RESTORE() \ + zend_fiber_switch_unblock(); \ BG(user_compare_fci) = old_user_compare_fci; \ BG(user_compare_fci_cache) = old_user_compare_fci_cache; \ @@ -6379,6 +6383,7 @@ PHP_FUNCTION(array_multisort) func[num_arrays - 1] = php_get_data_compare_func_unstable(sort_type, sort_order != PHP_SORT_ASC); bucket_compare_func_t *old_multisort_func = ARRAYG(multisort_func); ARRAYG(multisort_func) = func; + zend_fiber_switch_block(); /* Create the indirection array. This array is of size MxN, where * M is the number of entries in each input array and N is the number @@ -6470,6 +6475,7 @@ PHP_FUNCTION(array_multisort) efree(indirect); efree(func); efree(arrays); + zend_fiber_switch_unblock(); ARRAYG(multisort_func) = old_multisort_func; } /* }}} */ diff --git a/ext/standard/tests/array/array_sort_comparator_fiber_switch.phpt b/ext/standard/tests/array/array_sort_comparator_fiber_switch.phpt new file mode 100644 index 000000000000..77b2651cfb5d --- /dev/null +++ b/ext/standard/tests/array/array_sort_comparator_fiber_switch.phpt @@ -0,0 +1,125 @@ +--TEST-- +Sort functions: suspending a fiber from the comparator throws a FiberError +--FILE-- +start(); + echo "$label: no error\n"; + } catch (FiberError $e) { + echo "$label: ", $e->getMessage(), "\n"; + } +} + +$suspend = function ($x, $y) { + Fiber::suspend(); + return $x <=> $y; +}; + +run('usort', function () use ($suspend) { + $array = [3, 1, 2]; + usort($array, $suspend); +}); + +run('uasort', function () use ($suspend) { + $array = [3, 1, 2]; + uasort($array, $suspend); +}); + +run('uksort', function () use ($suspend) { + $array = ['b' => 1, 'a' => 2]; + uksort($array, $suspend); +}); + +run('array_udiff', function () use ($suspend) { + array_udiff([1, 2], [1], $suspend); +}); + +run('array_uintersect', function () use ($suspend) { + array_uintersect([1, 2], [1], $suspend); +}); + +run('array_diff_ukey', function () use ($suspend) { + array_diff_ukey(['a' => 1], ['a' => 1], $suspend); +}); + +run('array_multisort', function () { + $array = [ + new class { + function __toString(): string + { + Fiber::suspend(); + return '3'; + } + }, + new class { + function __toString(): string + { + return '1'; + } + }, + ]; + array_multisort($array, SORT_STRING); +}); + +$fiber = new Fiber(function () { + Fiber::suspend('before'); + $array = [3, 1, 2]; + usort($array, fn($x, $y) => $x <=> $y); + return $array; +}); +var_dump($fiber->start()); +$fiber->resume(); +var_dump($fiber->getReturn()); + +$array = [3, 1, 2]; +try { + usort($array, function ($x, $y) { + throw new Exception('boom'); + }); +} catch (Exception $e) { + echo $e->getMessage(), "\n"; +} +$fiber = new Fiber(function () { + Fiber::suspend('after exception'); +}); +var_dump($fiber->start()); + +$array = [3, 1, 2]; +usort($array, function ($x, $y) { + $inner = [2, 1]; + usort($inner, fn($a, $b) => $a <=> $b); + return $x <=> $y; +}); +var_dump($array); +?> +--EXPECT-- +usort: Cannot switch fibers in current execution context +uasort: Cannot switch fibers in current execution context +uksort: Cannot switch fibers in current execution context +array_udiff: Cannot switch fibers in current execution context +array_uintersect: Cannot switch fibers in current execution context +array_diff_ukey: Cannot switch fibers in current execution context +array_multisort: Cannot switch fibers in current execution context +string(6) "before" +array(3) { + [0]=> + int(1) + [1]=> + int(2) + [2]=> + int(3) +} +boom +string(15) "after exception" +array(3) { + [0]=> + int(1) + [1]=> + int(2) + [2]=> + int(3) +} From bd3f9b3aca890c0d20b794d94aa9f48afabe04a0 Mon Sep 17 00:00:00 2001 From: lacatoire Date: Tue, 20 Oct 2026 10:17:03 +0200 Subject: [PATCH 2/3] Keep the sort comparator per fiber instead of blocking fiber switches Blocking every fiber switch while a sort is running rejected code that works today, such as a single sort whose comparator suspends, or a sort that never calls its callback. The only thing that breaks is the LIFO restore of the request globals when sorts from different fibers interleave. Give each fiber its own copy of the comparison globals and swap them on every fiber switch with the fiber observers, so a sort that suspends keeps its callback and no switch is ever refused. --- ext/standard/array.c | 83 +++++++++++- ext/standard/php_array.h | 8 ++ .../array/array_sort_comparator_fiber.phpt | 116 ++++++++++++++++ .../array_sort_comparator_fiber_switch.phpt | 125 ------------------ 4 files changed, 201 insertions(+), 131 deletions(-) create mode 100644 ext/standard/tests/array/array_sort_comparator_fiber.phpt delete mode 100644 ext/standard/tests/array/array_sort_comparator_fiber_switch.phpt diff --git a/ext/standard/array.c b/ext/standard/array.c index 04790ffe4272..16dd9d8c6b4c 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -33,9 +33,11 @@ #include "zend_smart_str.h" #include "zend_bitset.h" #include "zend_exceptions.h" +#include "zend_extensions.h" +#include "zend_fibers.h" +#include "zend_observer.h" #include "ext/random/php_random.h" #include "zend_frameless_function.h" -#include "zend_fibers.h" /* {{{ defines */ @@ -68,10 +70,84 @@ static void php_array_init_globals(zend_array_globals *array_globals) } /* }}} */ +/* The user comparison callback is kept in request globals that the sort + * functions save and restore in LIFO order on the C stack. A fiber that + * suspends inside a comparison callback breaks that order, so every fiber + * gets its own copy of these globals, swapped in on each fiber switch. */ +static int php_array_fiber_rid = -1; + +static void php_array_fiber_state_save(php_array_fiber_state *state) +{ + state->user_compare_fci = BG(user_compare_fci); + state->user_compare_fci_cache = BG(user_compare_fci_cache); + state->multisort_func = ARRAYG(multisort_func); + state->compare_deprecation_thrown = ARRAYG(compare_deprecation_thrown); +} + +static void php_array_fiber_state_load(const php_array_fiber_state *state) +{ + if (state) { + BG(user_compare_fci) = state->user_compare_fci; + BG(user_compare_fci_cache) = state->user_compare_fci_cache; + ARRAYG(multisort_func) = state->multisort_func; + ARRAYG(compare_deprecation_thrown) = state->compare_deprecation_thrown; + } else { + /* A fiber that has not run any sort yet starts without a live comparison. */ + memset(&BG(user_compare_fci), 0, sizeof(BG(user_compare_fci))); + BG(user_compare_fci_cache) = empty_fcall_info_cache; + ARRAYG(multisort_func) = NULL; + ARRAYG(compare_deprecation_thrown) = 0; + } +} + +static php_array_fiber_state *php_array_fiber_state_of(zend_fiber_context *context) +{ + if (context == EG(main_fiber_context)) { + return &ARRAYG(main_fiber_state); + } + return context->reserved[php_array_fiber_rid]; +} + +static void php_array_fiber_init(zend_fiber_context *initializing) +{ + /* The reserved slots are not guaranteed to be zeroed. */ + initializing->reserved[php_array_fiber_rid] = NULL; +} + +static void php_array_fiber_switch(zend_fiber_context *from, zend_fiber_context *to) +{ + /* A finished fiber has no sort left to resume, there is nothing to keep. */ + if (from == EG(main_fiber_context) || from->status != ZEND_FIBER_STATUS_DEAD) { + php_array_fiber_state *state = php_array_fiber_state_of(from); + if (!state) { + state = emalloc(sizeof(php_array_fiber_state)); + from->reserved[php_array_fiber_rid] = state; + } + php_array_fiber_state_save(state); + } + + php_array_fiber_state_load(php_array_fiber_state_of(to)); +} + +static void php_array_fiber_destroy(zend_fiber_context *destroying) +{ + if (destroying->reserved[php_array_fiber_rid]) { + efree(destroying->reserved[php_array_fiber_rid]); + destroying->reserved[php_array_fiber_rid] = NULL; + } +} + PHP_MINIT_FUNCTION(array) /* {{{ */ { ZEND_INIT_MODULE_GLOBALS(array, php_array_init_globals, NULL); + php_array_fiber_rid = zend_get_resource_handle("standard_array"); + if (php_array_fiber_rid >= 0) { + zend_observer_fiber_init_register(php_array_fiber_init); + zend_observer_fiber_switch_register(php_array_fiber_switch); + zend_observer_fiber_destroy_register(php_array_fiber_destroy); + } + return SUCCESS; } /* }}} */ @@ -808,11 +884,8 @@ static int php_array_user_compare(Bucket *a, Bucket *b) /* {{{ */ old_user_compare_fci_cache = BG(user_compare_fci_cache); \ ARRAYG(compare_deprecation_thrown) = 0; \ BG(user_compare_fci_cache) = empty_fcall_info_cache; \ - /* The comparator is restored in LIFO order, which a fiber switch breaks. */ \ - zend_fiber_switch_block(); \ #define PHP_ARRAY_CMP_FUNC_RESTORE() \ - zend_fiber_switch_unblock(); \ BG(user_compare_fci) = old_user_compare_fci; \ BG(user_compare_fci_cache) = old_user_compare_fci_cache; \ @@ -6383,7 +6456,6 @@ PHP_FUNCTION(array_multisort) func[num_arrays - 1] = php_get_data_compare_func_unstable(sort_type, sort_order != PHP_SORT_ASC); bucket_compare_func_t *old_multisort_func = ARRAYG(multisort_func); ARRAYG(multisort_func) = func; - zend_fiber_switch_block(); /* Create the indirection array. This array is of size MxN, where * M is the number of entries in each input array and N is the number @@ -6475,7 +6547,6 @@ PHP_FUNCTION(array_multisort) efree(indirect); efree(func); efree(arrays); - zend_fiber_switch_unblock(); ARRAYG(multisort_func) = old_multisort_func; } /* }}} */ diff --git a/ext/standard/php_array.h b/ext/standard/php_array.h index a7e8afdec34b..f7a923500d6b 100644 --- a/ext/standard/php_array.h +++ b/ext/standard/php_array.h @@ -64,9 +64,17 @@ PHPAPI bool php_array_pick_keys(php_random_algo_with_state engine, zval *input, extern PHPAPI zend_class_entry *sort_direction_ce; +typedef struct _php_array_fiber_state { + zend_fcall_info user_compare_fci; + zend_fcall_info_cache user_compare_fci_cache; + bucket_compare_func_t *multisort_func; + bool compare_deprecation_thrown; +} php_array_fiber_state; + ZEND_BEGIN_MODULE_GLOBALS(array) bucket_compare_func_t *multisort_func; bool compare_deprecation_thrown; + php_array_fiber_state main_fiber_state; ZEND_END_MODULE_GLOBALS(array) #define ARRAYG(v) ZEND_MODULE_GLOBALS_ACCESSOR(array, v) diff --git a/ext/standard/tests/array/array_sort_comparator_fiber.phpt b/ext/standard/tests/array/array_sort_comparator_fiber.phpt new file mode 100644 index 000000000000..eeb8f6729911 --- /dev/null +++ b/ext/standard/tests/array/array_sort_comparator_fiber.phpt @@ -0,0 +1,116 @@ +--TEST-- +Sort functions: a comparator that suspends a fiber keeps its own callback +--FILE-- + $y; + }); + return $array; +}); +$fiber->start(); +while (!$fiber->isTerminated()) { + $fiber->resume(); +} +echo implode(',', $fiber->getReturn()), "\n"; + +echo "Another sort runs while the first one is suspended\n"; +$fiber = new Fiber(function () { + $array = [3, 1, 2]; + usort($array, function ($x, $y) { + echo "A\n"; + Fiber::suspend(); + return $x <=> $y; + }); + echo "A done\n"; +}); +$fiber->start(); +$array = [5, 4, 6]; +$resumed = false; +usort($array, function ($x, $y) use ($fiber, &$resumed) { + echo "B\n"; + if (!$resumed) { + $resumed = true; + $fiber->resume(); + } + return $x <=> $y; +}); +echo implode(',', $array), "\n"; + +echo "The suspended fiber is destroyed by another sort\n"; +$fiber = new Fiber(function () { + $array = [3, 1, 2]; + usort($array, function ($x, $y) { + Fiber::suspend(); + return $x <=> $y; + }); +}); +$fiber->start(); +$array = [5, 4, 6, 1, 2]; +usort($array, function ($x, $y) use (&$fiber) { + $fiber = null; + return $x <=> $y; +}); +echo implode(',', $array), "\n"; + +echo "Two array_multisort() calls suspend in __toString()\n"; +class Item +{ + public function __construct(private string $value) {} + + public function __toString(): string + { + Fiber::suspend(); + return $this->value; + } +} +$first = new Fiber(function () { + $array = [new Item('b'), new Item('a'), new Item('c')]; + array_multisort($array, SORT_STRING); + echo implode(',', array_map('strval', $array)) === 'a,b,c' ? "first sorted\n" : "first wrong\n"; +}); +$second = new Fiber(function () { + $array = [new Item('f'), new Item('e'), new Item('d')]; + array_multisort($array, SORT_STRING); + echo implode(',', array_map('strval', $array)) === 'd,e,f' ? "second sorted\n" : "second wrong\n"; +}); +$first->start(); +$second->start(); +while (!$first->isTerminated()) { + $first->resume(); +} +while (!$second->isTerminated()) { + $second->resume(); +} + +echo "A sort nested in a comparator\n"; +$array = [3, 1, 2]; +usort($array, function ($x, $y) { + $inner = [2, 1]; + usort($inner, fn($a, $b) => $a <=> $b); + return $x <=> $y; +}); +echo implode(',', $array), "\n"; + +?> +--EXPECT-- +Suspend and resume in a single sort +1,2,3 +Another sort runs while the first one is suspended +A +B +A +B +B +4,5,6 +The suspended fiber is destroyed by another sort +1,2,4,5,6 +Two array_multisort() calls suspend in __toString() +first sorted +second sorted +A sort nested in a comparator +1,2,3 diff --git a/ext/standard/tests/array/array_sort_comparator_fiber_switch.phpt b/ext/standard/tests/array/array_sort_comparator_fiber_switch.phpt deleted file mode 100644 index 77b2651cfb5d..000000000000 --- a/ext/standard/tests/array/array_sort_comparator_fiber_switch.phpt +++ /dev/null @@ -1,125 +0,0 @@ ---TEST-- -Sort functions: suspending a fiber from the comparator throws a FiberError ---FILE-- -start(); - echo "$label: no error\n"; - } catch (FiberError $e) { - echo "$label: ", $e->getMessage(), "\n"; - } -} - -$suspend = function ($x, $y) { - Fiber::suspend(); - return $x <=> $y; -}; - -run('usort', function () use ($suspend) { - $array = [3, 1, 2]; - usort($array, $suspend); -}); - -run('uasort', function () use ($suspend) { - $array = [3, 1, 2]; - uasort($array, $suspend); -}); - -run('uksort', function () use ($suspend) { - $array = ['b' => 1, 'a' => 2]; - uksort($array, $suspend); -}); - -run('array_udiff', function () use ($suspend) { - array_udiff([1, 2], [1], $suspend); -}); - -run('array_uintersect', function () use ($suspend) { - array_uintersect([1, 2], [1], $suspend); -}); - -run('array_diff_ukey', function () use ($suspend) { - array_diff_ukey(['a' => 1], ['a' => 1], $suspend); -}); - -run('array_multisort', function () { - $array = [ - new class { - function __toString(): string - { - Fiber::suspend(); - return '3'; - } - }, - new class { - function __toString(): string - { - return '1'; - } - }, - ]; - array_multisort($array, SORT_STRING); -}); - -$fiber = new Fiber(function () { - Fiber::suspend('before'); - $array = [3, 1, 2]; - usort($array, fn($x, $y) => $x <=> $y); - return $array; -}); -var_dump($fiber->start()); -$fiber->resume(); -var_dump($fiber->getReturn()); - -$array = [3, 1, 2]; -try { - usort($array, function ($x, $y) { - throw new Exception('boom'); - }); -} catch (Exception $e) { - echo $e->getMessage(), "\n"; -} -$fiber = new Fiber(function () { - Fiber::suspend('after exception'); -}); -var_dump($fiber->start()); - -$array = [3, 1, 2]; -usort($array, function ($x, $y) { - $inner = [2, 1]; - usort($inner, fn($a, $b) => $a <=> $b); - return $x <=> $y; -}); -var_dump($array); -?> ---EXPECT-- -usort: Cannot switch fibers in current execution context -uasort: Cannot switch fibers in current execution context -uksort: Cannot switch fibers in current execution context -array_udiff: Cannot switch fibers in current execution context -array_uintersect: Cannot switch fibers in current execution context -array_diff_ukey: Cannot switch fibers in current execution context -array_multisort: Cannot switch fibers in current execution context -string(6) "before" -array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) -} -boom -string(15) "after exception" -array(3) { - [0]=> - int(1) - [1]=> - int(2) - [2]=> - int(3) -} From 9f19ee1a1fccda1a17bd6df23aabb76988dbef61 Mon Sep 17 00:00:00 2001 From: lacatoire Date: Tue, 20 Oct 2026 10:26:23 +0200 Subject: [PATCH 3/3] Trim comments --- ext/standard/array.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 16dd9d8c6b4c..021089319efa 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -70,10 +70,7 @@ static void php_array_init_globals(zend_array_globals *array_globals) } /* }}} */ -/* The user comparison callback is kept in request globals that the sort - * functions save and restore in LIFO order on the C stack. A fiber that - * suspends inside a comparison callback breaks that order, so every fiber - * gets its own copy of these globals, swapped in on each fiber switch. */ +/* Sorts restore these globals in LIFO order, which fibers break: keep one copy per fiber. */ static int php_array_fiber_rid = -1; static void php_array_fiber_state_save(php_array_fiber_state *state) @@ -92,7 +89,6 @@ static void php_array_fiber_state_load(const php_array_fiber_state *state) ARRAYG(multisort_func) = state->multisort_func; ARRAYG(compare_deprecation_thrown) = state->compare_deprecation_thrown; } else { - /* A fiber that has not run any sort yet starts without a live comparison. */ memset(&BG(user_compare_fci), 0, sizeof(BG(user_compare_fci))); BG(user_compare_fci_cache) = empty_fcall_info_cache; ARRAYG(multisort_func) = NULL; @@ -116,7 +112,6 @@ static void php_array_fiber_init(zend_fiber_context *initializing) static void php_array_fiber_switch(zend_fiber_context *from, zend_fiber_context *to) { - /* A finished fiber has no sort left to resume, there is nothing to keep. */ if (from == EG(main_fiber_context) || from->status != ZEND_FIBER_STATUS_DEAD) { php_array_fiber_state *state = php_array_fiber_state_of(from); if (!state) {