`bug81577_3.phpt` pcntl with async signals leaks

[mvorisek]

Description

https://github.com/php/php-src/blob/master/ext/pcntl/tests/bug81577_3.phpt

https://bugs.php.net/bug.php?id=81577

Leaks are hard to figure in production, would be great if it could be fixed.

PHP Version

any

Operating System

linux

[derickr]

Hi @mvorisek — with bugsnet going to be turned off in the near future, it would be much better of you could reproduce the whole context of the bug here, including the test cases and what you have tried to find and fix the issue.

[bukka]

@mvorisek Technically the currently voted decision is to still use bugsnet for the old bugs so you should still leave comments there until it is decided otherwise. Although the spam is a bit pain so I would agree with Derick but only if you have some additional useful notes for that or a new way how to reproduce it then it would be more convenient to create a new bug report containing all info.

[mvorisek]

The memory leak can be reproduced consistently with https://github.com/php/php-src/blob/master/ext/pcntl/tests/bug81577_3.phpt test and ASAN. The original https://bugs.php.net/bug.php?id=81577 bug report seems to be fixed to no longer segfault but "only leaks memory".

[stkeke]

Memory Leak Root Cause

array_merge([1], [2]) + posix_kill(posix_getpid(), SIGTERM);

Simply put, the temporary variable returned by array_merge() does NOT get released after async signal handling until program exits. To fix, we should find a right place to destroy any temporary variables at right time.

Note: in normal program flow, these temporary variables will be used and consumed by later ADD opcode. But with async signal handling, normal execution flow is interrupted and redirected (jmp) to other opcode. Troubleshooting details see below.

Troubleshooting

The GDB commands used can be found at #10912.

php script

<?php
pcntl_async_signals(true);
pcntl_signal(SIGTERM, function ($signo) { throw new Exception("Signal"); });
try {
        array_merge([1], [2]) + posix_kill(posix_getpid(), SIGTERM);
} catch (Throwable $ex) {
        echo get_class($ex) , " : " , $ex->getMessage() , "\n";
}
?>

php opcode dump

V2 is created as array_merge() result. Without async signal handling, V2 is supposed to be consumed/destroyed by later ADD(L0018).

$_main:
     ; (lines=30, args=0, vars=1, tmps=3)
     ; (after optimizer)
     ; /home/tony/php-src/ext/pcntl/tests/bug81577_3.php:1-10
0000 INIT_FCALL 1 96 string("pcntl_async_signals")
...
0008 INIT_FCALL 2 112 string("array_merge")
  0009 SEND_VAL array(...) 1
  0010 SEND_VAL array(...) 2
/** V2 is created as array_merge() result. 
  * If everything is smooth, V2 is supposed to be consumed/destroyed by later ADD(L0018)
  * /
0011 V2 = DO_ICALL 

0012 INIT_FCALL 2 112 string("posix_kill")
    0013 INIT_FCALL 0 80 string("posix_getpid")
    0014 V1 = DO_ICALL # call posix_kill
0015 SEND_VAR V1 1
0016 SEND_VAL int(15) 2

/** V3 is created as posix_kill() result which is also supposed to be destroyed by later ADD(L0018) */
0017 V3 = DO_ICALL

0018 T1 = ADD V2 V3
0019 FREE T1
0020 RETURN int(1) /** $main function exit */

0021 CV0($ex) = CATCH string("Throwable")
0022 T1 = GET_CLASS CV0($ex)
0023 ECHO T1
0024 ECHO string(" : ")
0025 INIT_METHOD_CALL 0 CV0($ex) string("getMessage")
0026 V1 = DO_FCALL
0027 ECHO V1
0028 ECHO string("\n")
0029 RETURN int(1) /** exception handling */
LIVE RANGES:
     2: 0012 - 0018 (tmp/var)
EXCEPTION TABLE:
     0008, 0021, -, -

{closure}:
     ; (lines=5, args=1, vars=1, tmps=1)
     ; (after optimizer)
     ; /home/tony/php-src/ext/pcntl/tests/bug81577_3.php:3-3
0000 CV0($signo) = RECV 1
0001 V1 = NEW 1 string("Exception")
0002 SEND_VAL_EX string("Signal") 1
0003 DO_FCALL
0004 THROW V1
LIVE RANGES:
     1: 0002 - 0004 (new)

op_array dump

filename = < /home/tony/php-src/ext/pcntl/tests/bug81577_3.php:1-10 >
func=<Empty> type=[2]
[0] lineno=[2] opcode=[61] handler=[ 0x5555565736e2 <ZEND_INIT_FCALL_SPEC_CONST_HANDLER>]
[1] lineno=[2] opcode=[65] handler=[ 0x55555658248b <ZEND_SEND_VAL_SIMPLE_SPEC_CONST_HANDLER>]
[2] lineno=[2] opcode=[129] handler=[ 0x55555655b28a <ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER>]
[3] lineno=[3] opcode=[61] handler=[ 0x5555565736e2 <ZEND_INIT_FCALL_SPEC_CONST_HANDLER>]
[4] lineno=[3] opcode=[65] handler=[ 0x55555658248b <ZEND_SEND_VAL_SIMPLE_SPEC_CONST_HANDLER>]
[5] lineno=[3] opcode=[142] handler=[ 0x555556580422 <ZEND_DECLARE_LAMBDA_FUNCTION_SPEC_CONST_HANDLER>]
[6] lineno=[3] opcode=[65] handler=[ 0x5555565cece8 <ZEND_SEND_VAL_SPEC_TMPVAR_UNUSED_HANDLER>]
[7] lineno=[3] opcode=[129] handler=[ 0x55555655b28a <ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER>]
[8] lineno=[5] opcode=[61] handler=[ 0x5555565736e2 <ZEND_INIT_FCALL_SPEC_CONST_HANDLER>]
[9] lineno=[5] opcode=[65] handler=[ 0x55555658248b <ZEND_SEND_VAL_SIMPLE_SPEC_CONST_HANDLER>]
[10] lineno=[5] opcode=[65] handler=[ 0x55555658248b <ZEND_SEND_VAL_SIMPLE_SPEC_CONST_HANDLER>]
[11] lineno=[5] opcode=[129] handler=[ 0x55555655b9f7 <ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER>] // array_merge
[12] lineno=[5] opcode=[61] handler=[ 0x5555565736e2 <ZEND_INIT_FCALL_SPEC_CONST_HANDLER>]
[13] lineno=[5] opcode=[61] handler=[ 0x5555565736e2 <ZEND_INIT_FCALL_SPEC_CONST_HANDLER>]
[14] lineno=[5] opcode=[129] handler=[ 0x55555655b9f7 <ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER>]  // zif_posix_getpid
[15] lineno=[5] opcode=[117] handler=[ 0x55555661bcaa <ZEND_SEND_VAR_SPEC_VAR_UNUSED_HANDLER>]
[16] lineno=[5] opcode=[65] handler=[ 0x55555658248b <ZEND_SEND_VAL_SIMPLE_SPEC_CONST_HANDLER>]
[17] lineno=[5] opcode=[129] handler=[ 0x55555655b9f7 <ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER>] //  posix_kill 
				                         END_HANDLE_EXCEPTION_SPEC_HANDLER -> jump to [21] 
														
[18] lineno=[5] opcode=[1] handler=[ 0x5555565ad3ea <ZEND_ADD_SPEC_TMPVARCV_TMPVARCV_HANDLER>]
[19] lineno=[5] opcode=[70] handler=[ 0x5555565b69c2 <ZEND_FREE_SPEC_TMPVAR_HANDLER>]
[20] lineno=[10] opcode=[62] handler=[ 0x5555565782fa <ZEND_RETURN_SPEC_CONST_HANDLER>]

[21] lineno=[6] opcode=[107] handler=[ 0x55555657b187 <ZEND_CATCH_SPEC_CONST_HANDLER>]
[22] lineno=[7] opcode=[191] handler=[ 0x55555669694a <ZEND_GET_CLASS_SPEC_CV_UNUSED_HANDLER>]
[23] lineno=[7] opcode=[136] handler=[ 0x5555565b56f5 <ZEND_ECHO_SPEC_TMPVAR_HANDLER>]
[24] lineno=[7] opcode=[136] handler=[ 0x5555565770fa <ZEND_ECHO_SPEC_CONST_HANDLER>]
[25] lineno=[7] opcode=[112] handler=[ 0x55555666e02d <ZEND_INIT_METHOD_CALL_SPEC_CV_CONST_HANDLER>]
[26] lineno=[7] opcode=[60] handler=[ 0x55555655fd74 <ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER>]
[27] lineno=[7] opcode=[136] handler=[ 0x5555565b56f5 <ZEND_ECHO_SPEC_TMPVAR_HANDLER>]
[28] lineno=[7] opcode=[136] handler=[ 0x5555565770fa <ZEND_ECHO_SPEC_CONST_HANDLER>]
[29] lineno=[10] opcode=[62] handler=[ 0x5555565782fa <ZEND_RETURN_SPEC_CONST_HANDLER>]
                                                           -> zend_leave_helper_SPEC(ZEND_OPCODE_HANDLER_ARGS) 

Async Signal Handling Execution Flow Tracing

Let's look at op_array dump, after posix_kill calling, the program flow jumps to [21], then all the way down to [29], and finally program exits.
At least until [29], the arrange_merge() result V2 is still alive and in fact gets no chance to release even until program exits.

Here is the GDB debug tracing: break at ZEND_RETURN_SPEC_CONST_HANDLER and check all temporary variables. [0x7ffff0809090] is particularly of our interest, and its array is at [0x7ffff08025a0] (reported memory leak by PHP)

Breakpoint 4, ZEND_RETURN_SPEC_CONST_HANDLER () at /home/tony/php-src/Zend/zend_vm_execute.h:4375
4375            retval_ptr = RT_CONSTANT(opline, opline->op1);
(gdb) print_tmps
Stack frame address=[0x7ffff0809020]
Temporary variables count: 3

  0: [0x7ffff0809080] (refcount=2) string: Signal
  1: [0x7ffff0809090] (refcount=1) array:     Packed(2)[0x7ffff08025a0] flags=[0x14] pDestructor=[(dtor_func_t) 0x5555564ae8dd <zval_ptr_dtor>]
{
      [0] 0 => [0x7ffff0882488] long: 1
      [1] 1 => [0x7ffff0882498] long: 2
}
  
  2: [0x7ffff08090a0] bool: true

At program exit, PHP reports memory leak 0x00007ffff08025a0

(gdb) c
Continuing.
[Wed Mar 29 08:15:42 2023]  Script:  '/home/tony/php-src/ext/pcntl/tests/bug81577_3.php'
/home/tony/php-src/Zend/zend_hash.c(291) :  Freeing 0x00007ffff08025a0 (56 bytes), script=/home/tony/php-src/ext/pcntl/tests/bug81577_3.php
[Wed Mar 29 08:15:42 2023]  Script:  '/home/tony/php-src/ext/pcntl/tests/bug81577_3.php'
/home/tony/php-src/Zend/zend_hash.c(157) :  Freeing 0x00007ffff0882480 (136 bytes), script=/home/tony/php-src/ext/pcntl/tests/bug81577_3.php
=== Total 2 memory leaks detected ===

As for [0x00007ffff0882480], it is memory space used for HT_INVLID_INDEX and is just prior to the first array element [0x7ffff0882488]. And it is also belong to the unleased V2.