Edit report at https://bugs.php.net/bug.php?id=76601&edit=1
ID: 76601
Patch added by: mnikulin@plesk.com
Reported by: mnikulin at plesk dot com
Summary: Partially working php-fpm ater incomplete reload
Status: Assigned
Type: Bug
Package: FPM related
Operating System: Linux, Ubuntu-16.04
PHP Version: 7.0.30
Assigned To: bukka
Block user comment: N
Private report: N
New Comment:
The following pull request has been associated:
Patch Name: Do not let PHP-FPM children miss SIGTERM, SIGQUIT
On GitHub: https://github.com/php/php-src/pull/4836
Patch: https://github.com/php/php-src/pull/4836.patch
Previous Comments:
------------------------------------------------------------------------
[2019-02-24 17:04:33] bukka@php.net
Sorry for the late reply! Finally got time to take a look. First of all thanks for the thorough
analysis of the problem and all the patches!
The patches make sense from the first look but need to think about it more and do some testing.
In terms of the test, it needs some updates to the main tester to handle the main ini load (php-fpm
-c). Ideally it could be generated for each run with options to extend it. It would be also good to
have some helpers to quickly enable opcache as there are other issues that would make use of that. I
have got an idea how to do it and will try to find some time to implement it. It would be really
good to have this covered. It might help to minimize breaks to FPM that Opcache from time to time
introduces.
------------------------------------------------------------------------
[2019-02-20 04:05:04] mnikulin at plesk dot com
Related To: Bug #76895
------------------------------------------------------------------------
[2019-02-20 03:59:57] mnikulin at plesk dot com
As I said earlier, the easiest way to test
php-76601_kill-not-rescheduled_7.2.11_2018-09-28.patch
may be in the scope of Bug #76895. However I have realized that
to reproduce 76895 opcache must be enabled. I faced some
obstacles trying to implement such test
- PHP_INI_SCAN_DIR must be set to empty or invalid directory
otherwise .ini files there (from already installed previous build)
may change settings related error reporting and may lead
to failed expectations in messages tests.
- It is necessary to explicitly load opcache.so, so .ini file
should be passed to php-fpm command.
- opcache.so loading error must be ignored if it is not built.
- If built, opcache.so must be loaded from build directory,
and I have no idea now to obtain it.
I hope, somebody from PHP developers can solve such issues better than me.
Other reload failures are caused by races, so I am unsure it it is worth
to write tests that repeat actions many times due to such tests may lasts
quite long time.
I consider patches attached to this bug and to Bug #74083 as well tested
in production environment for php-5.6, and 7.0-7.3,
however the servers have mostly similar configuration.
I do not plan to work on the test further. My lates draft is
--TEST--
FPM: bug76895 bug77443 - child blocks reload
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php
require_once "tester.inc";
$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
pid = {{FILE:PID}}
[unconfined]
listen = {{ADDR}}
pm = ondemand
pm.max_children = 5
pm.start_servers = 1
pm.min_spare_servers = 1
pm.max_spare_servers = 1
EOT;
// TODO skip if opcache is not compiled
// TODO obtain build dir somehow
$cwd = getcwd();
$ini = <<<EOT
zend_extension = $cwd/build-fpm/modules/opcache.so
EOT;
$code = <<<EOT
<?php
\$variable = 'test';
if (!empty(\$variable)) {
break;
}
EOT;
// TODO It seems that tester.inc does not allow custom php.ini even though file can be created
$iniFile = __DIR__ . '/.user.ini';
putenv("PHPRC=$iniFile");
putenv("PHP_INI_SCAN_DIR=/dev/null");
$tester = new FPM\Tester($cfg, $code);
$tester->setUserIni($ini);
$tester->start();
$tester->expectLogStartNotices();
$body = $tester->request()->getBody();
$expectedBody = "/^<br \\/>\n<b>Fatal error<\\/b>: 'break' not in
the 'loop' or 'switch' context in <b>.*\.php<\\/b> on line
<b>4<\\/b><br \\/>$/";
if (!preg_match($expectedBody, $body)) {
echo 'ERROR: expected body ' . $expectedBody . "\n" . 'does not match
actual body: ' . $body . "\n";
false;
}
// Alternatively error may be suppressed in response and directed to log.
// $tester->expectLogWarning('child \d+ said into stderr: "NOTICE: PHP message: PHP
Fatal error: \'break\' not in the \'loop\' or \'switch\' context in
.* on line 4"', 'unconfined');
$tester->signal('USR2');
$tester->expectLogNotice('Reloading in progress ...');
$tester->expectLogNotice('reloading: .*');
$tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"');
$tester->expectLogStartNotices();
$tester->terminate();
$tester->expectLogTerminatingNotices();
$tester->close();
?>
Done
--EXPECT--
Done
--CLEAN--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>
------------------------------------------------------------------------
[2019-02-04 09:44:10] mnikulin at plesk dot com
Last week I was trying to write a test for Bug #76895 that is the easiest way to reproduce the case.
I realized that the code that runs test suite does not force empty TEST_PHP_EXECUTABLE directory so
if php is already installed to the prefix, ini files for modules may affect test results. I have not
reproduce the issue in the build environment but "break/continue" is reproducible on test
servers. The difference in modules/extensions and their configurations (sodium, etc.) and error
reporting options. My current draft is
-----
--TEST--
FPM: bug76895 bug77443 - child blocks reload
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php
require_once "tester.inc";
$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
pid = {{FILE:PID}}
[unconfined]
listen = {{ADDR}}
pm = ondemand
pm.max_children = 5
pm.start_servers = 1
pm.min_spare_servers = 1
pm.max_spare_servers = 1
catch_workers_output = yes
EOT;
$code = <<<EOT
<?php
\$variable = 'test';
if (!empty(\$variable)) {
break;
}
EOT;
$tester = new FPM\Tester($cfg, $code);
$tester->start();
$tester->expectLogStartNotices();
$tester->request()->expectEmptyBody();
$tester->expectLogWarning('child \d+ said into stderr: "NOTICE: PHP message: PHP Fatal
error: \'break\' not in the \'loop\' or \'switch\' context in .* on
line 4"', 'unconfined');
$tester->signal('USR2');
$tester->expectLogNotice('Reloading in progress ...');
$tester->expectLogNotice('reloading: .*');
$tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"');
$tester->expectLogStartNotices();
$tester->terminate();
$tester->expectLogTerminatingNotices();
$tester->close();
?>
Done
--EXPECT--
Done
--CLEAN--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>
-----
Message body is actually not empty in build environment, message
pattern and output should be adjusted as well.
I am unsure if I will have time this week to proceed further, hope the draft might be useful for
you.
------------------------------------------------------------------------
[2019-02-04 09:09:05] nikic@php.net
@bukka: Are you familiar with the FPM signal handling code? This looks like something we should
really fix and the patch at least looks reasonable to me.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=76601
--
Edit this bug report at https://bugs.php.net/bug.php?id=76601&edit=1