Fix: multi_curl_progress fires after shutdown on fatal error (ZTS FrankenPHP)

[AlliBalliBaba]

This issue was originally raised in the FrankenPHP repo php/frankenphp#2268. When a fatal timeout error happens during a curl_multi operation, the progress function apparently fires one last time when php has shut down, crashing the process.

Checking for shutdown in the curl_progress function fixes this (lines added in this PR). Would be better to fix the underlying issue that causes the function to fire after shutdown, not familiar with how the curl library works though. Happens only in ZTS.

To reproduce:

build and start at port 8087

   # move the debug.Dockerfile and index.php into the current working directory
   docker build -t frankenphp-debug -f debug.Dockerfile .

   # run the server
   docker run --rm  -p 8087:80 -v .:/app frankenphp-debug
   
   # from another shell:
   curl localhost:8087

debug.Dockerfile (FrankenPHP with PHP debug symbols)

# debug.Dockerfile
FROM dunglas/frankenphp

ARG PHP_VERSION=8.5

# install DEBUG version of PHP
RUN apt-get update && \
	apt-get -y --no-install-recommends install \
	$PHPIZE_DEPS \
	libargon2-dev \
	libbrotli-dev \
	libcurl4-openssl-dev \
	libonig-dev \
	libreadline-dev \
	libsodium-dev \
	libsqlite3-dev \
	libssl-dev \
	libxml2-dev \
	zlib1g-dev \
	bison \
	libnss3-tools \
	git \
	clang \
	cmake \
	llvm \
	gdb \
	valgrind \
	neovim \
	zsh \
	libtool-bin && \
	echo 'set auto-load safe-path /' > /root/.gdbinit && \
	echo '* soft core unlimited' >> /etc/security/limits.conf && \
	apt-get clean

WORKDIR /usr/local/src/php
RUN git clone --branch=PHP-${PHP_VERSION} https://github.com/php/php-src.git . && \
	./buildconf --force && \
	./configure \
		--enable-embed \
		--enable-zts \
		--disable-zend-signals \
		--enable-zend-max-execution-timers \
		--with-config-file-path=/usr/local/etc/php \
		--with-config-file-scan-dir=/usr/local/etc/php/conf.d \
		--enable-option-checking=fatal \
    	--with-mhash \
    	--with-pic \
   		--enable-mbstring \
    	--enable-mysqlnd \
		--with-password-argon2 \
		--with-sodium=shared \
		--with-pdo-sqlite=/usr \
    	--with-sqlite3=/usr \
    	--with-curl \
    	--with-iconv \
    	--with-openssl \
    	--with-readline \
    	--with-zlib \
    	--enable-phpdbg \
    	--enable-phpdbg-readline \
    	--with-pear \
    	--with-libdir="lib/$debMultiarch" \
		--enable-debug && \
	make -j"$(nproc)" && \
	make install && \
	ldconfig && \
	cp  /usr/local/etc/php/php.ini-development /usr/local/etc/php/php.ini && \
	php --version

RUN usermod -u 1000 www-data
RUN chown -R www-data:www-data /data/caddy /config/caddy /app

WORKDIR /app
USER www-data

ENTRYPOINT [ "frankenphp", "php-server", "--listen", ":80" ]

index.php (force a timeout with an active curl_progress function)

# index.php
<?php

set_time_limit(1);
$progress = function ($ch, $dltotal, $dlnow, $ultotal, $ulnow) {
    echo "Progress: $dltotal $dlnow $ultotal $ulnow\n";
    return 0;
};

$url = "http://1.1.1.1.1.1"; # hang forever here
$ch = curl_init();
curl_setopt($ch, CURLOPT_URL, $url);
curl_setopt($ch, CURLOPT_NOPROGRESS, false);
curl_setopt($ch, CURLOPT_PROGRESSFUNCTION, $progress);
$mh = curl_multi_init();
curl_multi_add_handle($mh, $ch);

$active = null;
do {
    $status = curl_multi_exec($mh, $active);
    if ($active) {
        curl_multi_select($mh);
    }
} while ($active && $status == CURLM_OK);

$result = curl_multi_getcontent($ch);
curl_multi_remove_handle($mh, $ch);
curl_multi_close($mh);

output

{"level":"error","ts":1773600887.4274793,"logger":"frankenphp","msg":"PHP Fatal error:  Maximum execution time of 1 second exceeded in /app/ctimeout.php on line 21\nStack trace:\n#0 /app/ctimeout.php(21): curl_multi_select(Object(CurlMultiHandle))\n#1 {main}","syslog_level":"err","stacktrace":"log/slog.(*Logger).LogAttrs\n\t/usr/local/go/src/log/slog/logger.go:194\ngithub.com/dunglas/frankenphp.go_log\n\t/go/src/app/frankenphp.go:714\n_cgoexp_d6966032c910_go_log\n\t/go/src/app/frankenphp.go:662\nruntime.cgocallbackg1\n\t/usr/local/go/src/runtime/cgocall.go:466\nruntime.cgocallbackg\n\t/usr/local/go/src/runtime/cgocall.go:362\nruntime.cgocallback\n\t/usr/local/go/src/runtime/asm_amd64.s:1160"}
{"level":"error","ts":1773600896.549346,"logger":"frankenphp","msg":"PHP Fatal error:  Couldn't execute method {closure:/app/ctimeout.php:4} in Unknown on line 0\nStack trace:\n#0 {main}","syslog_level":"err","stacktrace":"log/slog.(*Logger).LogAttrs\n\t/usr/local/go/src/log/slog/logger.go:194\ngithub.com/dunglas/frankenphp.go_log\n\t/go/src/app/frankenphp.go:714\n_cgoexp_d6966032c910_go_log\n\t/go/src/app/frankenphp.go:662\nruntime.cgocallbackg1\n\t/usr/local/go/src/runtime/cgocall.go:466\nruntime.cgocallbackg\n\t/usr/local/go/src/runtime/cgocall.go:362\nruntime.cgocallback\n\t/usr/local/go/src/runtime/asm_amd64.s:1160"}
/usr/local/src/php/main/main.c(1540) : Bailed out without a bailout address!
<br />
<b>Fatal error</b>:  Couldn't execute method {closure:/app/index.php:4} in <b>Unknown</b> on line <b>0</b><br />
Stack trace:
#0 {main}

[ndossche]

Trying to go to issue 2268 yields 404, please share a link.
Also this is a bad hack, do you have a reproducer?

[AlliBalliBaba]

Ah sry, here is the correct link: php/frankenphp#2268.

Yeah this is a bad hack, reproducer is in the description (of this PR). Copy the debug.Dockerfile and index.php from above into the same directory and run

docker build -t frankenphp-debug -f debug.Dockerfile .

# run the server
docker run --rm  -p 8087:80 -v .:/app frankenphp-debug
 
# from another shell:
curl localhost:8087

[ndossche]

That doesn't reproduce the issue for me. I just get this after I do the curl:

Progress: 0 0 0 0
Progress: 0 0 0 0
Progress: 0 0 0 0
Progress: 0 0 0 0
Progress: 0 0 0 0
Progress: 0 0 0 0

It's also unclear whether this is a FrankenPHP issue or a PHP issue.
Probably something should unregister the curl handles before executing shutdown code.

[AlliBalliBaba]

Hmm aren't the handlers unregistered in php_request_shutdown ? Could also be specific to FrankenPHP, not ZTS. Will look into it some more from that side. I'll leave this PR open in the meantime

[AlliBalliBaba]

Oh I just realized that php_request_shutdown() can also potentially bailout. FPM will probably just shut down the process if this happens. This is still a bug, but we can catch it on the FrankenPHP side I think.