php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #77934 php-fpm kill -USR2 not working
Submitted: 2019-04-24 09:54 UTC Modified: 2019-05-06 16:52 UTC
From: marc+bugsphp at cdmon dot com Assigned: bukka (profile)
Status: Closed Package: FPM related
PHP Version: 7.3.4 OS: Debian 9
Private report: No CVE-ID: None
 [2019-04-24 09:54 UTC] marc+bugsphp at cdmon dot com
Description:
------------
When you try to restart graceful php-fpm it holds untill process_control_timeout  triggers and sends TERM to each child.

This happens from version 7.1.20 and 7.2.8, the fix that brokes this was http://bugs.php.net/73342 the update was at 19 Jul 2018.

I was debugging the logs and found that the master process sends the SIGQUIT to the childs, and then no reply from childs to master happens since process_control_timeout stops:

>>> [24-Apr-2019 10:14:26.740642] DEBUG: pid 23148, fpm_pctl_kill_all(), line 157: [pool marctestqa.com] sending signal 3 SIGQUIT to child 23165

The logs stucks on fpm_event_loop for process_control_timeout time, then makes another fpm_pctl_kill_all but with SIGTERM:

>>> [24-Apr-2019 10:14:26.740697] DEBUG: pid 23148, fpm_event_loop(), line 417: event module triggered 1 events
>>> [24-Apr-2019 10:14:46.741631] DEBUG: pid 23148, fpm_pctl_kill_all(), line 157: [pool marctestqa.com] sending signal 15 SIGTERM to child 23165

Ok, fpm_event_loop is a timer that waits to the fpm_event_queue_fd:

>>> ret = module->wait(fpm_event_queue_fd, timeout); (line 409, file fpm_events.c, php 7.3.4)

I will suppose that on debian the module system is epoll, then module = fpm_event_epoll_module();, and then we have .wait = fpm_event_epoll_wait,

ret = fpm_event_epoll_module->fpm_event_epoll_wait(fd,t)

And I can't follow this because I don't have more Linux internals knowledge.




Test script:
---------------
edit php-fpm.conf and put some time into process_control_timeout:

[global]
process_control_timeout = 20s

Then open 2 terminals, in one call: watch -n0,1 "systemctl status php-fpm.service".

On the other term call: kill -USR2 PHPFPMPID*.

then see how it waits the 20 seconds and force the kill (SIGTERM) onto all the childrens. If you try this into 7.0 or prior to 7.2.8 or 7.1.20 it works.

*: PHPFPMPID: The current process ID of the Master.

Expected result:
----------------
A gradual restart of all the pools while they are freeing the sockets.

Actual result:
--------------
Wait till process_control_timeout, then SIGTERM all the pools.

Patches

fpm_kill_USR2_7.3.4_77934 (last revision 2019-05-06 09:20 UTC by marc+bugsphp at cdmon dot com)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-04-24 14:50 UTC] marc+bugsphp at cdmon dot com
I have modified the patch from 7.2.8 that it is this:

$ git diff php-7.2.7 php-7.2.8 -- sapi/fpm/fpm/fpm_children.c
diff --git a/sapi/fpm/fpm/fpm_children.c b/sapi/fpm/fpm/fpm_children.c
index b48fa54f53..4ee316ba1b 100644
--- a/sapi/fpm/fpm/fpm_children.c
+++ b/sapi/fpm/fpm/fpm_children.c
@@ -146,6 +146,7 @@ static struct fpm_child_s *fpm_child_find(pid_t pid) /* {{{ */
 static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
 {
        fpm_globals.max_requests = wp->config->pm_max_requests;
+       fpm_globals.listening_socket = dup(wp->listening_socket);

        if (0 > fpm_stdio_init_child(wp)  ||
            0 > fpm_log_init_child(wp)    ||


$ git diff php-7.2.7 php-7.2.8 -- sapi/fpm/fpm/fpm_stdio.c
diff --git a/sapi/fpm/fpm/fpm_stdio.c b/sapi/fpm/fpm/fpm_stdio.c
index 40720176e1..76e8b324df 100644
--- a/sapi/fpm/fpm/fpm_stdio.c
+++ b/sapi/fpm/fpm/fpm_stdio.c
@@ -103,12 +103,6 @@ int fpm_stdio_init_child(struct fpm_worker_pool_s *wp) /* {{{ */
        fpm_globals.error_log_fd = -1;
        zlog_set_fd(-1);

-       if (wp->listening_socket != STDIN_FILENO) {
-               if (0 > dup2(wp->listening_socket, STDIN_FILENO)) {
-                       zlog(ZLOG_SYSERROR, "failed to init child stdio: dup2()");
-                       return -1;
-               }
-       }
        return 0;
 }
 /* }}} */

and it worked as spected, restarting the childrens at moment of doing kill -USR2 PID. 

I need to add to this, that I'm using a 0 request server, is a test server to just test this bug.

Thanks.
 [2019-04-24 16:58 UTC] cmb@php.net
Do you mind submitting the patch as pull request[1], to ease
review and potential discussion.

[1] <https://github.com/php/php-src/pulls>
 [2019-04-25 06:37 UTC] marc+bugsphp at cdmon dot com
@Christoph the solution is just a rollback of a previous patch (http://git.php.net/?p=php-src.git;a=commitdiff;h=69dee5c732fe982c82edb17d0dbc3e79a47748d8), as I said I have deleted that patch and it worked. Maybe a workaround for me is just to delete that patch, but it's not the best scenario that I would like.
 [2019-04-28 18:21 UTC] bukka@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: bukka
 [2019-04-28 18:21 UTC] bukka@php.net
This is quite interesting that the mentioned fix made things worse for USR2 handling.

There are already some errors that could fix it for you too as there are some other issues with USR2 handling.

Would you able to test patches from the following bugs?:

https://bugs.php.net/bug.php?id=74083
https://bugs.php.net/bug.php?id=76601

There are patches for each bug. I will need to create a test for them before they can be merged but it would be great if they get more testing.
 [2019-04-29 10:06 UTC] marc+bugsphp at cdmon dot com
Dear Jakub, I have tested both patches toghether and it didn't solved the problem. Sorry.
 [2019-05-05 15:11 UTC] bukka@php.net
I have been looking to the fpm_signals.c and noticed that we still try to close standard input which might cause this issue. It's done in sig_soft_quit function. So maybe this could fix it:

diff --git a/sapi/fpm/fpm/fpm_signals.c b/sapi/fpm/fpm/fpm_signals.c
index 1b97025ea7..caf41e8a3f 100644
--- a/sapi/fpm/fpm/fpm_signals.c
+++ b/sapi/fpm/fpm/fpm_signals.c
@@ -142,7 +142,7 @@ static void sig_soft_quit(int signo) /* {{{ */
        int saved_errno = errno;
 
        /* closing fastcgi listening socket will force fcgi_accept() exit immediately */
-       close(0);
+       close(fpm_globals.listening_socket);
        if (0 > socket(AF_UNIX, SOCK_STREAM, 0)) {
                zlog(ZLOG_WARNING, "failed to create a new socket");
        }
 [2019-05-06 09:17 UTC] marc+bugsphp at cdmon dot com
Yeah Jakub! that easy to solve! That worked perfect for me, I have compiled a 
7.3.4 and tested right now and it worked as spected.

Now we need that someone more try this or will you push it to next revisions?

Thanks!!
 [2019-05-06 16:52 UTC] nikic@php.net
@bukka: Nice catch!
 [2019-05-11 19:07 UTC] bukka@php.net
Automatic comment on behalf of bukka
Revision: http://git.php.net/?p=php-src.git;a=commit;h=cc5c51e7f0732067f105d13c6d355fcab5965c2f
Log: Fix bug #77934 (php-fpm kill -USR2 not working)
 [2019-05-11 19:07 UTC] bukka@php.net
-Status: Assigned +Status: Closed
 [2019-05-11 19:09 UTC] bukka@php.net
Automatic comment on behalf of bukka
Revision: http://git.php.net/?p=php-src.git;a=commit;h=cc5c51e7f0732067f105d13c6d355fcab5965c2f
Log: Fix bug #77934 (php-fpm kill -USR2 not working)
 [2019-05-11 19:10 UTC] bukka@php.net
Automatic comment on behalf of bukka
Revision: http://git.php.net/?p=php-src.git;a=commit;h=cc5c51e7f0732067f105d13c6d355fcab5965c2f
Log: Fix bug #77934 (php-fpm kill -USR2 not working)
 [2019-05-11 19:10 UTC] bukka@php.net
Automatic comment on behalf of bukka
Revision: http://git.php.net/?p=php-src.git;a=commit;h=cc5c51e7f0732067f105d13c6d355fcab5965c2f
Log: Fix bug #77934 (php-fpm kill -USR2 not working)
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Tue Jun 18 05:01:27 2019 UTC