8.2.2: segfault when garbage collector is invoked inside of fiber

[danog]

Description

Most likely caused by the changes in #9810

The following code:

<?php

use danog\MadelineProto\EventHandler;
use danog\MadelineProto\Logger;
use danog\MadelineProto\Settings;

include 'vendor/autoload.php';

class SecretHandler extends EventHandler
{
}

$settings = new Settings;
$settings->getLogger()->setLevel(Logger::ULTRA_VERBOSE);
$settings->getAppInfo()->setApiId(6)->setApiHash('eb06d4abfb49dc3eeb1aeb98ae0f581e');

SecretHandler::startAndLoop('secret.madeline', $settings);

With this composer.json:

{
	"require": {"danog/madelineproto": "8.0.0-beta44"},
	"minimum-stability": "dev"
}

Resulted in a segfault, gdb backtrace:

#0  0x00005555558e58df in zend_unfinished_execution_gc (execute_data=0x7ffff4d7e1f0, call=0xd00000000, gc_buffer=0x5555568042e8 <executor_globals+1640>)
    at /usr/src/debug/php/php-8.2.2/Zend/zend_execute.c:4419
#1  0x0000555555969890 in zend_fiber_object_gc (object=<optimized out>, table=0x7fffe6bfdcd8, num=0x7fffe6bfdcd4) at /usr/src/debug/php/php-8.2.2/Zend/zend_fibers.c:672
#2  0x000055555594b168 in gc_mark_grey (stack=0x7fffe6bfdce0, ref=<optimized out>) at /usr/src/debug/php/php-8.2.2/Zend/zend_gc.c:832
#3  gc_mark_roots (stack=0x7fffe6bfdce0) at /usr/src/debug/php/php-8.2.2/Zend/zend_gc.c:986
#4  zend_gc_collect_cycles () at /usr/src/debug/php/php-8.2.2/Zend/zend_gc.c:1497
#5  0x00005555558dc1bc in zif_gc_collect_cycles (execute_data=<optimized out>, return_value=0x7fffe6bfed50) at /usr/src/debug/php/php-8.2.2/Zend/zend_builtin_functions.c:93
#6  0x00005555558eac20 in ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER (execute_data=0x7fffe6468130) at /usr/src/debug/php/php-8.2.2/Zend/zend_vm_execute.h:1250
#7  0x0000555555937248 in execute_ex (ex=<optimized out>) at /usr/src/debug/php/php-8.2.2/Zend/zend_vm_execute.h:55811
#8  0x00005555558b36de in zend_call_function (fci=<optimized out>, fci_cache=<optimized out>) at /usr/src/debug/php/php-8.2.2/Zend/zend_execute_API.c:930
#9  0x000055555596960f in zend_fiber_execute (transfer=0x7fffe6bfefb0) at /usr/src/debug/php/php-8.2.2/Zend/zend_fibers.c:504
#10 0x0000555555969392 in zend_fiber_trampoline (data=...) at /usr/src/debug/php/php-8.2.2/Zend/zend_fibers.c:299
#11 0x0000555555a03b17 in make_fcontext () at make_x86_64_sysv_elf_gas.S:78
#12 0x0000000000000000 in ?? ()

gdb zbacktrace:

(gdb) zbacktrace
[0x7fffe64681e0] gc_collect_cycles() [internal function]
[0x7fffe6468130] danog\MadelineProto\GarbageCollector->danog\MadelineProto\{closure}(6) /tmp/a/vendor/danog/madelineproto/src/GarbageCollector.php:53
[0x7fffe6468070] Revolt\EventLoop\Internal\AbstractDriver->Revolt\EventLoop\Internal\{closure}() /tmp/a/vendor/revolt/event-loop/src/EventLoop/Internal/AbstractDriver.php:577
[0x7fffe6468020] (main) [internal function]
[0x7ffff4d7e170] Fiber->resume() [internal function]
[0x7ffff4d7e0f0] Revolt\EventLoop\Internal\AbstractDriver->invokeCallbacks() /tmp/a/vendor/revolt/event-loop/src/EventLoop/Internal/AbstractDriver.php:478
[0x7ffff4d7e070] Revolt\EventLoop\Internal\AbstractDriver->Revolt\EventLoop\Internal\{closure}() /tmp/a/vendor/revolt/event-loop/src/EventLoop/Internal/AbstractDriver.php:533
[0x7ffff4d7e020] (main) [internal function]
[0x7ffff4c14ed0] Fiber->resume() [internal function]
[0x7ffff4c14e60] Revolt\EventLoop\Internal\AbstractDriver->Revolt\EventLoop\Internal\{closure}() /tmp/a/vendor/revolt/event-loop/src/EventLoop/Internal/AbstractDriver.php:86
[0x7ffff4c14d60] Revolt\EventLoop\Internal\DriverSuspension->suspend() /tmp/a/vendor/revolt/event-loop/src/EventLoop/Internal/DriverSuspension.php:97
[0x7ffff4c14c90] Amp\Future->await(NULL) /tmp/a/vendor/amphp/amp/src/Future.php:251
[0x7ffff4c14c10] danog\MadelineProto\WrappedFuture->await() /tmp/a/vendor/danog/madelineproto/src/WrappedFuture.php:57
[0x7ffff4c14b60] danog\MadelineProto\Connection->methodCallAsyncRead("req_DH_params", array(6)[0x7ffff4c14bc0]) /tmp/a/vendor/danog/madelineproto/src/MTProtoSession/CallHandler.php:104
[0x7ffff4c14700] danog\MadelineProto\Connection->createAuthKey(false) /tmp/a/vendor/danog/madelineproto/src/MTProtoSession/AuthKeyHandler.php:200
[0x7ffff4c145c0] danog\MadelineProto\DataCenterConnection->initAuthorization() /tmp/a/vendor/danog/madelineproto/src/DataCenterConnection.php:162
[0x7ffff4c14500] danog\MadelineProto\MTProto->initAuthorization() /tmp/a/vendor/danog/madelineproto/src/MTProtoTools/AuthKeyHandler.php:46
[0x7ffff4c14430] danog\MadelineProto\MTProto->connectToAllDcs() /tmp/a/vendor/danog/madelineproto/src/MTProto.php:1364
[0x7ffff4c14360] danog\MadelineProto\MTProto->initialize(object[0x7ffff4c143b0]) /tmp/a/vendor/danog/madelineproto/src/MTProto.php:639
[0x7ffff4c142b0] danog\MadelineProto\MTProto->__construct(object[0x7ffff4c14300], object[0x7ffff4c14310]) /tmp/a/vendor/danog/madelineproto/src/MTProto.php:584
[0x7ffff4c14120] danog\MadelineProto\API->__construct("secret.madeline", object[0x7ffff4c14180]) /tmp/a/vendor/danog/madelineproto/src/API.php:184
[0x7ffff4c14090] danog\MadelineProto\EventHandler->startAndLoop("secret.madeline", object[0x7ffff4c140f0]) /tmp/a/vendor/danog/madelineproto/src/EventHandler.php:47
[0x7ffff4c14020] (main) /tmp/a/a.php:38

The same happens even without forcefully invoking gc_collect_cycles, as soon the automatic garbage collection mechanism triggers.

PHP Version

PHP 8.2.2

Operating System

Arch linux

[danog]

This happens even without invoking gc_collect_cycles, during normal automatic garbage collection.
This completely breaks the garbage collector in php 8.2.2 when using fibers.

[bwoebi]

A simple reproducer I just found - there seems to be a bug in zend_unfinished_execution_gc in that it tries to add the args of an already running function.

$ ~/php-src-8.2/sapi/cli/php -r 'function x(&$ref) { } function y($x) { Fiber::suspend(); } $f = new Fiber(function() use (&$f) { x($var); \ord(y(1)); }); $f->start(); unset($f); gc_collect_cycles();'
php: /root/php-src-8.2/Zend/zend_types.h:1228: zend_gc_delref: Assertion `p->refcount > 0' failed.
Aborted (core dumped)

I'll have a closer look now.

[bwoebi]

@arnaud-lb Essentially: zend_unfinished_calls_gc is working fine with generators as there are no child calls, i.e. the current EX(call) is what a ZEND_SEND_VAL at EX(opline) - 1 will have added args to.

However, with fibers, there may be an active child frame, thus EX(opline) - 1 refers to the current executing child frame - and not EX(call) - which is where it goes wrong. Essentially, in fibers, we need to skip the current call: In Fibers suspensions always happen within child frames (within a Fiber::suspend() or a function calling that somewhere further down the call stack), with generators they happen in the current frame (i.e. on yield).

I'm right now not exactly sure on the specifics, but essentially, I think we need to pass a bool to zend_unfinished_calls_gc depending on whether it's called from generator or fiber context and then conditionally skip the current function.

[hooker007]

If I have php 8.2.2, how can I switch to 8.2.1 in debian?

[danog]

Hang on, the PR didn't fix the issue, just checked with a freshly compiled php-src (e8d16fd).

Also, the test merged along with the PR doesn't segfault even on PHP 8.2.2 on my arch machine.

[bwoebi]

There's indeed a second issue:

function pollute_stack_and_suspend($a = 1, $b = 2, $c = 3) {
    Fiber::suspend();
}
$f = new Fiber(function() use (&$f) {
    pollute_stack_and_suspend();
    (function() {
        (function() {
            (new Fiber(function() {
                gc_collect_cycles();
            }))->start();
        })();
    })();
});
$f->start();
$f->resume();

[bwoebi]

This happens as suspending writes fiber->execute_data back, but switching to another fiber does not.
This was not a problem in the past as a fiber could not destroyed while being caller for another fiber, so that was irrelevant.

Added an explicit check for get_gc here.

[danog]

Thanks!

Do you think you could tag an RC for 8.2.4 and 8.1.17?

Fibers are completely broken for everyone right now on the latest stables, and will be until those fixes are tagged.

[hoseiinb123]

Pleas speed up the release of PHP 8.1.17 or PHP 8.2.4, For fixing this Bug.

[Elovel]

Please speed up the release of PHP 8.1.17 and 8.2.4 to fix this issue...

[PUXLBIY]

Please speed up the release of PHP 8.1.17 and 8.2.4 to fix this problem

[arnaud-lb]

I'm checking with release managers

[koloner]

Please speed up the release of PHP 8.1.17 and 8.2.4 to fix this problem

[damianwadley]

Spamming requests to "speed up" the releases is almost as helpful as honking in traffic. Let's stop that, alright?

[botyoutobe]

يرجى تسريع إصدار PHP 8.1.17 و 8.2.4 لإصلاح هذه المشكلة

[XtoCv]

Please speed up the release of PHP 8.1.17 and 8.2.4 to fix this problem

[bwoebi]

Locking this issue to reduce spam. @arnaud-lb however, do you actually have heard back from the RMs?

[arnaud-lb]

I heard back from RMs: new RCs will be tagged on Tuesday next week and released on Thursday