Skip to content

Fix fiber switch corrupting user comparator globals in sort functions - #24426

Open
lacatoire wants to merge 3 commits into
php:masterfrom
lacatoire:fix/sort-comparator-fiber-switch-upstream
Open

Fix fiber switch corrupting user comparator globals in sort functions#24426
lacatoire wants to merge 3 commits into
php:masterfrom
lacatoire:fix/sort-comparator-fiber-switch-upstream

Conversation

@lacatoire

@lacatoire lacatoire commented Oct 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

usort(), uasort(), uksort(), array_multisort() and the user-callback array_diff()/array_intersect() variants keep the comparison callback in request globals and restore them in LIFO order. When a comparator suspends a fiber and sorts from different fibers interleave, the restore writes back a stale value: the wrong comparator runs, or the engine crashes.

Each fiber now gets its own copy of these globals, swapped on every fiber switch, so suspending from a comparator keeps working and no switch is refused.

To reproduce on PHP 8.4

<?php
$fiber = new Fiber(function () {
    $a = [3, 1, 2];
    usort($a, function ($x, $y) { echo "A cmp\n"; Fiber::suspend(); return $x <=> $y; });
    echo "A done\n";
});
$fiber->start();
$b = [5, 4, 6];
$once = false;
usort($b, function ($x, $y) use ($fiber, &$once) {
    echo "B cmp\n";
    if (!$once) { $once = true; $fiber->resume(); }
    return $x <=> $y;
});
echo "end\n";

Before: A cmp, B cmp, B cmp, B cmp, A done (the fiber ran B's comparator), then Fatal error: Uncaught Error: Invalid callback , no array or string given

After: A cmp, B cmp, A cmp, B cmp, B cmp, end

@lacatoire
Fix fiber switch corrupting user comparator globals in sort functions
edfe692 
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.
@lacatoire
lacatoire requested a review from bukka as a code owner October 19, 2026 11:47
ndossche
ndossche requested changes Oct 19, 2026
View reviewed changes

@ndossche ndossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annoying, but it is what it is. Are these the only of family functions that do this?
I think the fix is incomplete though:

  • Even if no callback is fired, fiber switching is still blocked. That may break working code.
  • Related to the first bullet point: suspending and resuming works fine as long as there's no nested calls, yet fiber blocking is done anyway
  • array_udiff_assoc & array_uintersect_assoc have the same issue

@lacatoire lacatoire mentioned this pull request Oct 20, 2026
Open
@lacatoire
Keep the sort comparator per fiber instead of blocking fiber switches
bd3f9b3 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ndossche ndossche ndossche requested changes
@bukka bukka Awaiting requested review from bukka bukka is a code owner

Assignees

No one assigned

Labels

ABI break Extension: standard

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lacatoire @ndossche