From: Nikita Popov <nikita.ppv@gmail.com>
Date: Tue, 23 Jul 2019 13:54:35 +0000 (+0200)
Subject: Fix FPM timer event re-registration
X-Git-Tag: php-7.4.0beta2~17
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0ed6c37140;p=php

Fix FPM timer event re-registration

Make sure that fpm_event_add calls inside a timer callback work by
unregistering the event from the queue before invoking its callback.

The read timeout in tester.inc is increased because the added test
needs two seconds (one for SIGTERM, one for SIGKILL) until the
reload succeeds, so we should wait longer than that for a response.
---

diff --git a/sapi/fpm/fpm/fpm_events.c b/sapi/fpm/fpm/fpm_events.c
index 81db9f8867..86592050c6 100644
--- a/sapi/fpm/fpm/fpm_events.c
+++ b/sapi/fpm/fpm/fpm_events.c
@@ -433,17 +433,16 @@ void fpm_event_loop(int err) /* {{{ */
 		/* trigger timers */
 		q = fpm_event_queue_timer;
 		while (q) {
+			struct fpm_event_queue_s *next = q->next;
 			fpm_clock_get(&now);
 			if (q->ev) {
 				if (timercmp(&now, &q->ev->timeout, >) || timercmp(&now, &q->ev->timeout, ==)) {
-					fpm_event_fire(q->ev);
-					/* sanity check */
-					if (fpm_globals.parent_pid != getpid()) {
-						return;
-					}
-					if (q->ev->flags & FPM_EV_PERSIST) {
-						fpm_event_set_timeout(q->ev, now);
-					} else { /* delete the event */
+					struct fpm_event_s *ev = q->ev;
+					if (ev->flags & FPM_EV_PERSIST) {
+						fpm_event_set_timeout(ev, now);
+					} else {
+						/* Delete the event. Make sure this happens before it is fired,
+						 * so that the event callback may register the same timer again. */
 						q2 = q;
 						if (q->prev) {
 							q->prev->next = q->next;
@@ -457,13 +456,18 @@ void fpm_event_loop(int err) /* {{{ */
 								fpm_event_queue_timer->prev = NULL;
 							}
 						}
-						q = q->next;
 						free(q2);
-						continue;
+					}
+
+					fpm_event_fire(ev);
+
+					/* sanity check */
+					if (fpm_globals.parent_pid != getpid()) {
+						return;
 					}
 				}
 			}
-			q = q->next;
+			q = next;
 		}
 	}
 }
diff --git a/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt b/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt
new file mode 100644
index 0000000000..25c2cf25e2
--- /dev/null
+++ b/sapi/fpm/tests/reload-uses-sigkill-as-last-measure.phpt
@@ -0,0 +1,56 @@
+--TEST--
+If SIGQUIT and SIGTERM during reloading fail, SIGKILL should be sent
+--SKIPIF--
+<?php
+include "skipif.inc";
+if (!function_exists('pcntl_sigprocmask')) die('skip Requires pcntl_sigprocmask()');
+?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+pid = {{FILE:PID}}
+process_control_timeout=1
+[unconfined]
+listen = {{ADDR}}
+ping.path = /ping
+ping.response = pong
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 1
+pm.min_spare_servers = 1
+pm.max_spare_servers = 1
+EOT;
+
+$code = <<<EOT
+<?php
+pcntl_sigprocmask(SIG_BLOCK, [SIGQUIT, SIGTERM]);
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+$tester->request()->expectEmptyBody();
+$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->ping('{{ADDR}}');
+$tester->terminate();
+$tester->expectLogTerminatingNotices();
+$tester->close();
+
+?>
+Done
+--EXPECT--
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
diff --git a/sapi/fpm/tests/tester.inc b/sapi/fpm/tests/tester.inc
index 12fe3dc2de..8d17db2e60 100644
--- a/sapi/fpm/tests/tester.inc
+++ b/sapi/fpm/tests/tester.inc
@@ -623,7 +623,7 @@ class Tester
         $read = [$this->outDesc];
         $write = null;
         $except = null;
-        if (stream_select($read, $write, $except, 2 )) {
+        if (stream_select($read, $write, $except, 3)) {
             return fgets($this->outDesc);
         } else {
             return null;