Postpone signal delivery while spawning children.
Prevent the following case:
- Reload (reexec) is in progress.
- New master is forking to start enough children for pools
where `pm` is not `on-demand`.
- Another `SIGUSR2` is received by the master process.
- Master process switches to reloading state.
- Some child has not set its own signal handlers.
- `SIGQUIT` and `SIGTERM` sent by master process are caught
by signal handler set by master process and so they are ignored.
- A child is running, it has no reason to finish
Before pull request #4465 this scenario could cause deadlock,
however with
0ed6c37140 reload finishes after `SIGKILL`.
Use sigprocmask() around fork() to avoid race of delivery signal to children
and setting of own signal handlers.
Fixes bug #76601
return 2;
}
+ zlog(ZLOG_DEBUG, "blocking signals before child birth");
+ if (0 > fpm_signals_child_block()) {
+ zlog(ZLOG_WARNING, "child may miss signals");
+ }
+
pid = fork();
switch (pid) {
return 0;
case -1 :
+ zlog(ZLOG_DEBUG, "unblocking signals");
+ fpm_signals_unblock();
zlog(ZLOG_SYSERROR, "fork() failed");
fpm_resources_discard(child);
return 2;
default :
+ zlog(ZLOG_DEBUG, "unblocking signals, child born");
+ fpm_signals_unblock();
child->pid = pid;
fpm_clock_get(&child->started);
fpm_parent_resources_use(child);
does that for us! thies@thieso.net
20000419 */
- /* Subset of signals from fpm_signals_init_main() to avoid unexpected death during early init
- or during reload just after execvp() or fork */
- int init_signal_array[] = { SIGUSR1, SIGUSR2, SIGCHLD };
- if (0 > fpm_signals_init_mask(init_signal_array, sizeof(init_signal_array)/sizeof(init_signal_array[0])) ||
- 0 > fpm_signals_block()) {
+ if (0 > fpm_signals_init_mask() || 0 > fpm_signals_block()) {
zlog(ZLOG_WARNING, "Could die in the case of too early reload signal");
}
zlog(ZLOG_DEBUG, "Blocked some signals");
static int sp[2];
static sigset_t block_sigset;
+static sigset_t child_block_sigset;
const char *fpm_signal_names[NSIG + 1] = {
#ifdef SIGHUP
int saved_errno;
if (fpm_globals.parent_pid != getpid()) {
- /* prevent a signal race condition when child process
- have not set up it's own signal handler yet */
+ /* Avoid using of signal handlers from the master process in a worker
+ before the child sets up its own signal handlers.
+ Normally it is prevented by the sigprocmask() calls
+ around fork(). This execution branch is a last resort trap
+ that has no protection against #76601. */
return;
}
}
zend_signal_init();
+
+ if (0 > fpm_signals_unblock()) {
+ return -1;
+ }
return 0;
}
/* }}} */
}
/* }}} */
-int fpm_signals_init_mask(int *signum_array, size_t size) /* {{{ */
+int fpm_signals_init_mask() /* {{{ */
{
+ /* Subset of signals from fpm_signals_init_main() and fpm_got_signal()
+ blocked to avoid unexpected death during early init
+ or during reload just after execvp() or fork */
+ int init_signal_array[] = { SIGUSR1, SIGUSR2, SIGCHLD };
+ size_t size = sizeof(init_signal_array)/sizeof(init_signal_array[0]);
size_t i = 0;
- if (0 > sigemptyset(&block_sigset)) {
+ if (0 > sigemptyset(&block_sigset) ||
+ 0 > sigemptyset(&child_block_sigset)) {
zlog(ZLOG_SYSERROR, "failed to prepare signal block mask: sigemptyset()");
return -1;
}
for (i = 0; i < size; ++i) {
- int sig_i = signum_array[i];
- if (0 > sigaddset(&block_sigset, sig_i)) {
+ int sig_i = init_signal_array[i];
+ if (0 > sigaddset(&block_sigset, sig_i) ||
+ 0 > sigaddset(&child_block_sigset, sig_i)) {
if (sig_i <= NSIG && fpm_signal_names[sig_i] != NULL) {
zlog(ZLOG_SYSERROR, "failed to prepare signal block mask: sigaddset(%s)",
fpm_signal_names[sig_i]);
return -1;
}
}
+ if (0 > sigaddset(&child_block_sigset, SIGTERM) ||
+ 0 > sigaddset(&child_block_sigset, SIGQUIT)) {
+ zlog(ZLOG_SYSERROR, "failed to prepare child signal block mask: sigaddset()");
+ return -1;
+ }
return 0;
}
/* }}} */
}
/* }}} */
+int fpm_signals_child_block() /* {{{ */
+{
+ if (0 > sigprocmask(SIG_BLOCK, &child_block_sigset, NULL)) {
+ zlog(ZLOG_SYSERROR, "failed to block child signals");
+ return -1;
+ }
+ return 0;
+}
+/* }}} */
+
int fpm_signals_unblock() /* {{{ */
{
/* Ensure that during reload after upgrade all signals are unblocked.
int fpm_signals_init_main();
int fpm_signals_init_child();
int fpm_signals_get_fd();
-int fpm_signals_init_mask(int *signum_array, size_t size);
+int fpm_signals_init_mask();
int fpm_signals_block();
+int fpm_signals_child_block();
int fpm_signals_unblock();
extern const char *fpm_signal_names[NSIG + 1];
--- /dev/null
+--TEST--
+FPM: bug76601 children should not ignore signals during concurrent reloads
+--SKIPIF--
+<?php
+include "skipif.inc";
+if (getenv("SKIP_SLOW_TESTS")) die("skip slow test");
+?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+pid = {{FILE:PID}}
+; some value twice greater than tester->getLogLines() timeout
+process_control_timeout=10
+[unconfined]
+listen = {{ADDR}}
+; spawn children immediately after reload
+pm = static
+pm.max_children = 10
+EOT;
+
+$code = <<<EOT
+<?php
+/* empty */
+EOT;
+
+/*
+ * If a child miss SIGQUIT then reload process should stuck
+ * for at least process_control_timeout that is set greater
+ * than timout in log reading functions.
+ *
+ * Alternative way is to set log_level=debug and filter result of
+ * $tester->getLogLines(2000) for lines containing SIGKILL
+ *
+ * [22-Oct-2019 03:28:19.532703] DEBUG: pid 21315, fpm_pctl_kill_all(), line 161: [pool unconfined] sending signal 9 SIGKILL to child 21337
+ * [22-Oct-2019 03:28:19.533471] DEBUG: pid 21315, fpm_children_bury(), line 259: [pool unconfined] child 21337 exited on signal 9 (SIGKILL) after 1.003055 seconds from start
+ *
+ * but it has less probability of failure detection. Additionally it requires more
+ * $tester->expectLogNotice() around last reload due to presence of debug messages.
+ */
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+
+/* Vary interval between concurrent reload requests
+ since performance of test instance is not known in advance */
+$max_interval = 25000;
+$step = 1000;
+$pid = $tester->getPid();
+for ($interval = 0; $interval < $max_interval; $interval += $step) {
+ exec("kill -USR2 $pid", $out, $killExitCode);
+ if ($killExitCode) {
+ echo "ERROR: master process is dead\n";
+ break;
+ }
+ usleep($interval);
+}
+echo "Reached interval $interval us with $step us steps\n";
+$tester->expectLogNotice('Reloading in progress ...');
+/* Consume mix of 'Reloading in progress ...' and 'reloading: .*' */
+$skipped = $tester->getLogLines(2000);
+
+$tester->signal('USR2');
+$tester->expectLogNotice('Reloading in progress ...');
+/* When a child ignores SIGQUIT, the following expectation fails due to timeout. */
+if (!$tester->expectLogNotice('reloading: .*')) {
+ /* for troubleshooting */
+ echo "Skipped messages\n";
+ echo implode('', $skipped);
+}
+$tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"');
+$tester->expectLogStartNotices();
+
+$tester->terminate();
+$tester->expectLogTerminatingNotices();
+$tester->close();
+
+?>
+Done
+--EXPECT--
+Reached interval 25000 us with 1000 us steps
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
projects / php / commitdiff