Skip to content

Commit 0ed6c37

Browse files
committed
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.
1 parent 6913ec3 commit 0ed6c37

File tree

3 files changed

+72
-12
lines changed

3 files changed

+72
-12
lines changed

sapi/fpm/fpm/fpm_events.c

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -433,17 +433,16 @@ void fpm_event_loop(int err) /* {{{ */
433433
/* trigger timers */
434434
q = fpm_event_queue_timer;
435435
while (q) {
436+
struct fpm_event_queue_s *next = q->next;
436437
fpm_clock_get(&now);
437438
if (q->ev) {
438439
if (timercmp(&now, &q->ev->timeout, >) || timercmp(&now, &q->ev->timeout, ==)) {
439-
fpm_event_fire(q->ev);
440-
/* sanity check */
441-
if (fpm_globals.parent_pid != getpid()) {
442-
return;
443-
}
444-
if (q->ev->flags & FPM_EV_PERSIST) {
445-
fpm_event_set_timeout(q->ev, now);
446-
} else { /* delete the event */
440+
struct fpm_event_s *ev = q->ev;
441+
if (ev->flags & FPM_EV_PERSIST) {
442+
fpm_event_set_timeout(ev, now);
443+
} else {
444+
/* Delete the event. Make sure this happens before it is fired,
445+
* so that the event callback may register the same timer again. */
447446
q2 = q;
448447
if (q->prev) {
449448
q->prev->next = q->next;
@@ -457,13 +456,18 @@ void fpm_event_loop(int err) /* {{{ */
457456
fpm_event_queue_timer->prev = NULL;
458457
}
459458
}
460-
q = q->next;
461459
free(q2);
462-
continue;
460+
}
461+
462+
fpm_event_fire(ev);
463+
464+
/* sanity check */
465+
if (fpm_globals.parent_pid != getpid()) {
466+
return;
463467
}
464468
}
465469
}
466-
q = q->next;
470+
q = next;
467471
}
468472
}
469473
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
--TEST--
2+
If SIGQUIT and SIGTERM during reloading fail, SIGKILL should be sent
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc";
6+
if (!function_exists('pcntl_sigprocmask')) die('skip Requires pcntl_sigprocmask()');
7+
?>
8+
--FILE--
9+
<?php
10+
11+
require_once "tester.inc";
12+
13+
$cfg = <<<EOT
14+
[global]
15+
error_log = {{FILE:LOG}}
16+
pid = {{FILE:PID}}
17+
process_control_timeout=1
18+
[unconfined]
19+
listen = {{ADDR}}
20+
ping.path = /ping
21+
ping.response = pong
22+
pm = dynamic
23+
pm.max_children = 5
24+
pm.start_servers = 1
25+
pm.min_spare_servers = 1
26+
pm.max_spare_servers = 1
27+
EOT;
28+
29+
$code = <<<EOT
30+
<?php
31+
pcntl_sigprocmask(SIG_BLOCK, [SIGQUIT, SIGTERM]);
32+
EOT;
33+
34+
$tester = new FPM\Tester($cfg, $code);
35+
$tester->start();
36+
$tester->expectLogStartNotices();
37+
$tester->request()->expectEmptyBody();
38+
$tester->signal('USR2');
39+
$tester->expectLogNotice('Reloading in progress ...');
40+
$tester->expectLogNotice('reloading: .*');
41+
$tester->expectLogNotice('using inherited socket fd=\d+, "127.0.0.1:\d+"');
42+
$tester->expectLogStartNotices();
43+
$tester->ping('{{ADDR}}');
44+
$tester->terminate();
45+
$tester->expectLogTerminatingNotices();
46+
$tester->close();
47+
48+
?>
49+
Done
50+
--EXPECT--
51+
Done
52+
--CLEAN--
53+
<?php
54+
require_once "tester.inc";
55+
FPM\Tester::clean();
56+
?>

sapi/fpm/tests/tester.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ class Tester
623623
$read = [$this->outDesc];
624624
$write = null;
625625
$except = null;
626-
if (stream_select($read, $write, $except, 2 )) {
626+
if (stream_select($read, $write, $except, 3)) {
627627
return fgets($this->outDesc);
628628
} else {
629629
return null;

0 commit comments

Comments
 (0)