Entering shutdown sequence with a fiber suspended in a Generator emits an unavoidable fatal error or crashes

[bwoebi]

Description

The following code:

<?php
$gen = (function() {
    try {
        Fiber::suspend();
        yield;
    } finally {
        echo "Finally\n";
    }
})();
$fiber = new Fiber(function() use ($gen, &$fiber) {
    $gen->current();
});
$fiber->start();

Resulted in this output (in Unknown on line 0 is also very helpful to track the issue down...):

Fatal error: Cannot resume an already running generator in Unknown on line 0

But I expected this output instead:

Finally

It seems necessary to add some ordering constraint within the interaction of fibers and generators. (Swapping the creation of the generator and the fiber in this example poses no problems.)

Similarly there is a crash in a slight variation of this code (without finally):

$gen = (function() {
    Fiber::suspend();
    yield;
})();
$fiber = new Fiber(function() use ($gen, &$fiber) {
    $gen->current();
});
$fiber->start();

Resulting in:

==1123727==ERROR: AddressSanitizer: heap-use-after-free on address 0x60b0000024b0 at pc 0x55f810df3b7b bp 0x7f10c723c2e0 sp 0x7f10c723c2d0
READ of size 8 at 0x60b0000024b0 thread T0
    #0 0x55f810df3b7a in execute_ex (/root/php-src-X/sapi/cli/php+0x55f3b7a)
    #1 0x55f810f6be1f in zend_generator_resume (/root/php-src-X/sapi/cli/php+0x576be1f)
    #2 0x55f810f6d4b0 in zend_generator_ensure_initialized (/root/php-src-X/sapi/cli/php+0x576d4b0)
    #3 0x55f810f6dd30 in zim_Generator_current (/root/php-src-X/sapi/cli/php+0x576dd30)
    #4 0x55f810df2531 in execute_ex (/root/php-src-X/sapi/cli/php+0x55f2531)
    #5 0x55f81052499b in zend_call_function (/root/php-src-X/sapi/cli/php+0x4d2499b)
    #6 0x55f8110cfa1a in zend_fiber_execute (/root/php-src-X/sapi/cli/php+0x58cfa1a)
    #7 0x55f8110cb9d7 in zend_fiber_trampoline (/root/php-src-X/sapi/cli/php+0x58cb9d7)
    #8 0x55f81024aa26 in make_fcontext (/root/php-src-X/sapi/cli/php+0x4a4aa26)

0x60b0000024b0 is located 0 bytes inside of 112-byte region [0x60b0000024b0,0x60b000002520)
freed by thread T0 here:
    #0 0x7f10ce2e0517 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x55f8104127e2 in _efree_custom (/root/php-src-X/sapi/cli/php+0x4c127e2)
    #2 0x55f810427a5b in _efree (/root/php-src-X/sapi/cli/php+0x4c27a5b)
    #3 0x55f810f5b73e in zend_generator_close (/root/php-src-X/sapi/cli/php+0x575b73e)
    #4 0x55f810f5cbc3 in zend_generator_dtor_storage (/root/php-src-X/sapi/cli/php+0x575cbc3)
    #5 0x55f81103a653 in zend_objects_store_call_destructors (/root/php-src-X/sapi/cli/php+0x583a653)
    #6 0x55f81050ef39 in shutdown_destructors (/root/php-src-X/sapi/cli/php+0x4d0ef39)
    #7 0x55f8105defa0 in zend_call_destructors (/root/php-src-X/sapi/cli/php+0x4ddefa0)
    #8 0x55f81025b62e in php_request_shutdown (/root/php-src-X/sapi/cli/php+0x4a5b62e)
    #9 0x55f81149e3f8 in do_cli (/root/php-src-X/sapi/cli/php+0x5c9e3f8)
    #10 0x55f81149f3d3 in main (/root/php-src-X/sapi/cli/php+0x5c9f3d3)
    #11 0x7f10cd0dcd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

previously allocated by thread T0 here:
    #0 0x7f10ce2e0867 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x55f8104366bf in __zend_malloc (/root/php-src-X/sapi/cli/php+0x4c366bf)
    #2 0x55f81041274b in _malloc_custom (/root/php-src-X/sapi/cli/php+0x4c1274b)
    #3 0x55f8104272f3 in _emalloc (/root/php-src-X/sapi/cli/php+0x4c272f3)
    #4 0x55f8108467ae in ZEND_GENERATOR_CREATE_SPEC_HANDLER (/root/php-src-X/sapi/cli/php+0x50467ae)
    #5 0x55f810df6ea9 in execute_ex (/root/php-src-X/sapi/cli/php+0x55f6ea9)
    #6 0x55f810e9406d in zend_execute (/root/php-src-X/sapi/cli/php+0x569406d)
    #7 0x55f81052d2eb in zend_eval_stringl (/root/php-src-X/sapi/cli/php+0x4d2d2eb)
    #8 0x55f81052dd73 in zend_eval_stringl_ex (/root/php-src-X/sapi/cli/php+0x4d2dd73)
    #9 0x55f81052de8d in zend_eval_string_ex (/root/php-src-X/sapi/cli/php+0x4d2de8d)
    #10 0x55f81149ba21 in do_cli (/root/php-src-X/sapi/cli/php+0x5c9ba21)
    #11 0x55f81149f3d3 in main (/root/php-src-X/sapi/cli/php+0x5c9f3d3)
    #12 0x7f10cd0dcd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

PHP Version

master

Operating System

No response

[bwoebi]

I am wondering whether it fundamentally makes sense to enforce some ordering of destruction order: first all fibers are destructed, then all other objects.
Makes in my eyes also most sense given that fibers are most likely to trigger some other code execution.
And given that, in destructors, no other fiber may be created, this should be safe.

An alternative fix approach could be delaying running the destruction of generators which are marked as currently running.

[cmb69]

According to https://3v4l.org/DDlsM, this affects PHP 8.1+.

[trowski]

@bwoebi You mentioned this does not affect debug builds in the linked issue, why is that?

[bwoebi]

@trowski The bug also does affect debug builds, but possibly by chance something was slightly different (e.g. order in object store, I don't know) in debug build.

[andypost]

This test has failed for 8.1.17RC1 and 8.2.4RC1 on ppc64le #10512 (comment)