Patch SmallerPatchKeepsEdgeLevelAndChildrenTellParentsMore for FPM related Bug #69724
Patch version 2016-11-24 16:47 UTC
Return to Bug #69724 |
Download this patch
Patch Revisions:
Developer: nick@cpanel.net
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: "J. Nick Koston" <nick@cpanel.net>
Date: Wed, 24 Aug 2016 17:23:43 -0500
Subject: [PATCH 5/5] ONDEMAND fix from
https://bugs.php.net/bug.php?id=69724
ONDEMAND Fix part2
CPANEL-9788
Only mark the child as accepted if the accept()
actually has success.
---
SOURCES/php/sapi/fpm/fpm/fastcgi.c | 47 +++++++++++++++++++++----
SOURCES/php/sapi/fpm/fpm/fastcgi.h | 3 +-
SOURCES/php/sapi/fpm/fpm/fpm_children.c | 19 +++++++++-
SOURCES/php/sapi/fpm/fpm/fpm_events.c | 18 ++++++----
SOURCES/php/sapi/fpm/fpm/fpm_main.c | 12 ++++++-
SOURCES/php/sapi/fpm/fpm/fpm_process_ctl.c | 56 +++++++++++++++++++++++++++++-
SOURCES/php/sapi/fpm/fpm/fpm_process_ctl.h | 3 ++
SOURCES/php/sapi/fpm/fpm/fpm_request.c | 18 ++++++++++
SOURCES/php/sapi/fpm/fpm/fpm_request.h | 2 ++
SOURCES/php/sapi/fpm/fpm/fpm_worker_pool.h | 4 ++-
10 files changed, 164 insertions(+), 18 deletions(-)
diff --git a/SOURCES/php/sapi/fpm/fpm/fastcgi.c b/SOURCES/php/sapi/fpm/fpm/fastcgi.c
index 778119e..437568f 100644
--- a/SOURCES/php/sapi/fpm/fpm/fastcgi.c
+++ b/SOURCES/php/sapi/fpm/fpm/fastcgi.c
@@ -803,7 +803,15 @@ static int fcgi_is_allowed() {
return 0;
}
-int fcgi_accept_request(fcgi_request *req)
+/**
+ * Accept a connect and parse a FastCGI request.
+ *
+ * @param req: the request structure to fill in
+ * @param accept_pipe: if non-negative, write the pid to this fd every time
+ * accept() returns and the fd still has more to read
+ * @returns the accepted fd, or -1 on error.
+ */
+int fcgi_accept_request(fcgi_request *req, int accept_pipe)
{
#ifdef _WIN32
HANDLE pipe;
@@ -845,12 +853,24 @@ int fcgi_accept_request(fcgi_request *req)
#endif
sa_t sa;
socklen_t len = sizeof(sa);
-
- fpm_request_accepting();
-
- FCGI_LOCK(req->listen_socket);
- req->fd = accept(listen_socket, (struct sockaddr *)&sa, &len);
- FCGI_UNLOCK(req->listen_socket);
+ fpm_request_accepting();
+ FCGI_LOCK(req->listen_socket);
+ req->fd = accept(listen_socket, (struct sockaddr *)&sa, &len);
+ if (req->fd < 0) {
+ zlog(ZLOG_DEBUG, "Child pid %d lost the accept() race with errno %d, retrying", getpid(), errno);
+ } else {
+ fpm_request_accepted(); // Mark ourselves as no longer ACCEPTING
+ if (accept_pipe >= 0 && fcgi_can_read(listen_socket) > 0) {
+ /* At this point we have another request waiting on the wire
+ * and we need to tell the parent that we did not get them all
+ * so it can see if another child is needed */
+ pid_t pid = getpid();
+ if (write(accept_pipe, &pid, sizeof(pid)) != sizeof(pid)) {
+ zlog(ZLOG_ERROR, "Failed writing to accept_pipe; something bad is going to happen.");
+ }
+ }
+ }
+ FCGI_UNLOCK(req->listen_socket);
client_sa = sa;
if (req->fd >= 0 && !fcgi_is_allowed()) {
@@ -1126,6 +1146,19 @@ void fcgi_free_mgmt_var_cb(void * ptr)
pefree(*var, 1);
}
+int fcgi_can_read (int listen_socket) {
+ fd_set rfds;
+ struct timeval tv;
+ int retval;
+
+ FD_ZERO(&rfds);
+ FD_SET(listen_socket, &rfds);
+
+ tv.tv_sec = 0;
+ tv.tv_usec = 0;
+ return select(1, &rfds, NULL, NULL, &tv);
+}
+
const char *fcgi_get_last_client_ip() /* {{{ */
{
static char str[INET6_ADDRSTRLEN];
diff --git a/SOURCES/php/sapi/fpm/fpm/fastcgi.h b/SOURCES/php/sapi/fpm/fpm/fastcgi.h
index 4da1c35..a01360c 100644
--- a/SOURCES/php/sapi/fpm/fpm/fastcgi.h
+++ b/SOURCES/php/sapi/fpm/fpm/fastcgi.h
@@ -115,8 +115,9 @@ typedef struct _fcgi_request {
int fcgi_init(void);
void fcgi_shutdown(void);
void fcgi_init_request(fcgi_request *req, int listen_socket);
-int fcgi_accept_request(fcgi_request *req);
+int fcgi_accept_request(fcgi_request *req, int accept_pipe);
int fcgi_finish_request(fcgi_request *req, int force_close);
+int fcgi_can_read (int listen_socket);
void fcgi_set_in_shutdown(int);
void fcgi_set_allowed_clients(char *);
diff --git a/SOURCES/php/sapi/fpm/fpm/fpm_children.c b/SOURCES/php/sapi/fpm/fpm/fpm_children.c
index 45cc075..fe82f9e 100644
--- a/SOURCES/php/sapi/fpm/fpm/fpm_children.c
+++ b/SOURCES/php/sapi/fpm/fpm/fpm_children.c
@@ -147,7 +147,8 @@ static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
{
fpm_globals.max_requests = wp->config->pm_max_requests;
- if (0 > fpm_stdio_init_child(wp) ||
+ if (0 > fpm_pctl_init_child(wp) ||
+ 0 > fpm_stdio_init_child(wp) ||
0 > fpm_log_init_child(wp) ||
0 > fpm_status_init_child(wp) ||
0 > fpm_unix_init_child(wp) ||
@@ -448,6 +449,22 @@ int fpm_children_create_initial(struct fpm_worker_pool_s *wp) /* {{{ */
wp->socket_event_set = 1;
fpm_event_add(wp->ondemand_event, 0);
+ if (pipe(wp->child_accept_pipe) < 0) {
+ zlog(ZLOG_ERROR, "[pool %s] unable to create child_accept_pipe", wp->config->name);
+ // FIXME handle crash
+ return 1;
+ }
+
+ wp->child_accept_event = (struct fpm_event_s *)malloc(sizeof(struct fpm_event_s));
+ if (!wp->child_accept_event) {
+ zlog(ZLOG_ERROR, "[pool %s] unable to malloc the child_accept socket event", wp->config->name);
+ // FIXME handle crash
+ return 1;
+ }
+ memset(wp->child_accept_event, 0, sizeof(struct fpm_event_s));
+ fpm_event_set(wp->child_accept_event, wp->child_accept_pipe[0], FPM_EV_READ, fpm_pctl_on_child_accept, wp);
+ fpm_event_add(wp->child_accept_event, 0);
+
return 1;
}
return fpm_children_make(wp, 0 /* not in event loop yet */, 0, 1);
diff --git a/SOURCES/php/sapi/fpm/fpm/fpm_events.c b/SOURCES/php/sapi/fpm/fpm/fpm_events.c
index ce5d543..e5f5798 100644
--- a/SOURCES/php/sapi/fpm/fpm/fpm_events.c
+++ b/SOURCES/php/sapi/fpm/fpm/fpm_events.c
@@ -526,12 +526,18 @@ int fpm_event_add(struct fpm_event_s *ev, unsigned long int frequency) /* {{{ */
int fpm_event_del(struct fpm_event_s *ev) /* {{{ */
{
- if (ev->index >= 0 && fpm_event_queue_del(&fpm_event_queue_fd, ev) != 0) {
- return -1;
- }
-
- if (ev->index < 0 && fpm_event_queue_del(&fpm_event_queue_timer, ev) != 0) {
- return -1;
+ if ( ev->index >= 0 ) {
+ if (fpm_event_queue_del(&fpm_event_queue_fd, ev) != 0) {
+ return -1;
+ }
+ /* the queue del will set the index to -1
+ * This used to call the timer code if
+ * index < 0 and thus always return -1 if
+ * fpm_event_queue_del set ev->index to -1 */
+ } else {
+ if (fpm_event_queue_del(&fpm_event_queue_timer, ev) != 0) {
+ return -1;
+ }
}
return 0;
diff --git a/SOURCES/php/sapi/fpm/fpm/fpm_main.c b/SOURCES/php/sapi/fpm/fpm/fpm_main.c
index d12ac01..13d1980 100644
--- a/SOURCES/php/sapi/fpm/fpm/fpm_main.c
+++ b/SOURCES/php/sapi/fpm/fpm/fpm_main.c
@@ -1913,7 +1913,17 @@ consult the installation file that came with this distribution, or visit \n\
fcgi_init_request(&request, fcgi_fd);
zend_first_try {
- while (fcgi_accept_request(&request) >= 0) {
+ // TODO: There has to be a better way to pass this info from the parent
+ // to the child. I should be able to access the fpm_worker_pool_s.
+ int pm, accept_pipe[2];
+ fpm_pctl_child_info(&pm, accept_pipe);
+ int child_accept_fd = -1;
+ if (pm == PM_STYLE_ONDEMAND) {
+ // TODO: move this close to some setup routine somewhere
+ close(accept_pipe[0]);
+ child_accept_fd = accept_pipe[1];
+ }
+ while (fcgi_accept_request(&request, child_accept_fd) >= 0) {
char *primary_script = NULL;
request_body_fd = -1;
SG(server_context) = (void *) &request;
diff --git a/SOURCES/php/sapi/fpm/fpm/fpm_process_ctl.c b/SOURCES/php/sapi/fpm/fpm/fpm_process_ctl.c
index 76ea4d3..9ab471f 100644
--- a/SOURCES/php/sapi/fpm/fpm/fpm_process_ctl.c
+++ b/SOURCES/php/sapi/fpm/fpm/fpm_process_ctl.c
@@ -520,12 +520,23 @@ void fpm_pctl_on_socket_accept(struct fpm_event_s *ev, short which, void *arg) /
}
for (child = wp->children; child; child = child->next) {
- /* if there is at least on idle child, it will handle the connection, stop here */
+ /* if there is at least one idle child, it will handle the connection, stop here */
if (fpm_request_is_idle(child)) {
return;
}
}
+ if (wp->can_read_before_forking) {
+ wp->can_read_before_forking = 0;
+ zlog(ZLOG_DEBUG, "[pool %s] checking can read", wp->config->name);
+ if (!fcgi_can_read(wp->listening_socket)) {
+ zlog(ZLOG_DEBUG, "[pool %s] fcgi could not read on the listening socket", wp->config->name);
+ return;
+ } else {
+ zlog(ZLOG_DEBUG, "[pool %s] fcgi could reado n the listening socket, child will be made", wp->config->name);
+ };
+ }
+
wp->warn_max_children = 0;
fpm_children_make(wp, 1, 1, 1);
@@ -537,3 +548,46 @@ void fpm_pctl_on_socket_accept(struct fpm_event_s *ev, short which, void *arg) /
}
/* }}} */
+void fpm_pctl_on_child_accept(struct fpm_event_s *ev, short which, void *arg) /* {{{ */
+{
+ struct fpm_worker_pool_s *wp = (struct fpm_worker_pool_s *)arg;
+ pid_t pid;
+
+ if (fpm_globals.parent_pid != getpid()) {
+ /* prevent a event race condition when child process
+ * have not set up its own event loop */
+ return;
+ }
+
+ // TODO: Isn't this redundant with the check above?
+ if (fpm_globals.is_child) {
+ return;
+ }
+
+ if (read(wp->child_accept_pipe[0], &pid, sizeof(pid)) == sizeof(pid)) {
+ zlog(ZLOG_DEBUG, "[pool %s] could still read after accept() in child, checking to see if we need another child.", wp->config->name);
+ fpm_pctl_on_socket_accept(ev, which, arg);
+ } else {
+ zlog(ZLOG_ERROR, "[pool %s] error %d reading child accept pipe", wp->config->name, errno);
+ }
+
+}
+/* }}} */
+
+// TODO: There has to be a better way to pass this info from the parent
+// to the child. I should be able to access the fpm_worker_pool_s.
+int fpm_pctl_child_pm, fpm_pctl_child_accept_pipe[2];
+int fpm_pctl_init_child(struct fpm_worker_pool_s *wp) /* {{{ */
+{
+ fpm_pctl_child_pm = wp->config->pm;
+ memcpy(fpm_pctl_child_accept_pipe, wp->child_accept_pipe, sizeof(fpm_pctl_child_accept_pipe));
+ return 0;
+}
+/* }}} */
+
+void fpm_pctl_child_info(int *pm, int *pipe) /* {{{ */
+{
+ *pm = fpm_pctl_child_pm;
+ memcpy(pipe, fpm_pctl_child_accept_pipe, sizeof(fpm_pctl_child_accept_pipe));
+}
+/* }}} */
diff --git a/SOURCES/php/sapi/fpm/fpm/fpm_process_ctl.h b/SOURCES/php/sapi/fpm/fpm/fpm_process_ctl.h
index 86a6ef0..0ff9249 100644
--- a/SOURCES/php/sapi/fpm/fpm/fpm_process_ctl.h
+++ b/SOURCES/php/sapi/fpm/fpm/fpm_process_ctl.h
@@ -24,9 +24,12 @@ void fpm_pctl_kill_all(int signo);
void fpm_pctl_heartbeat(struct fpm_event_s *ev, short which, void *arg);
void fpm_pctl_perform_idle_server_maintenance_heartbeat(struct fpm_event_s *ev, short which, void *arg);
void fpm_pctl_on_socket_accept(struct fpm_event_s *ev, short which, void *arg);
+void fpm_pctl_on_child_accept(struct fpm_event_s *ev, short which, void *arg);
int fpm_pctl_child_exited();
int fpm_pctl_init_main();
+int fpm_pctl_init_child(struct fpm_worker_pool_s *wp);
+void fpm_pctl_child_info(int *pm, int *pipe);
enum {
FPM_PCTL_STATE_UNSPECIFIED,
diff --git a/SOURCES/php/sapi/fpm/fpm/fpm_request.c b/SOURCES/php/sapi/fpm/fpm/fpm_request.c
index ed7e7a8..600098a 100644
--- a/SOURCES/php/sapi/fpm/fpm/fpm_request.c
+++ b/SOURCES/php/sapi/fpm/fpm/fpm_request.c
@@ -58,6 +58,24 @@ void fpm_request_accepting() /* {{{ */
}
/* }}} */
+void fpm_request_accepted() /* {{{ */
+{
+ struct fpm_scoreboard_proc_s *proc;
+ struct timeval now;
+
+ fpm_clock_get(&now);
+
+ proc = fpm_scoreboard_proc_acquire(NULL, -1, 0);
+ if (proc == NULL) {
+ zlog(ZLOG_WARNING, "failed to acquire proc scoreboard");
+ return;
+ }
+
+ proc->request_stage = FPM_REQUEST_ACCEPTED;
+ fpm_scoreboard_proc_release(proc);
+}
+/* }}} */
+
void fpm_request_reading_headers() /* {{{ */
{
struct fpm_scoreboard_proc_s *proc;
diff --git a/SOURCES/php/sapi/fpm/fpm/fpm_request.h b/SOURCES/php/sapi/fpm/fpm/fpm_request.h
index aebd36c..f68206b 100644
--- a/SOURCES/php/sapi/fpm/fpm/fpm_request.h
+++ b/SOURCES/php/sapi/fpm/fpm/fpm_request.h
@@ -19,9 +19,11 @@ void fpm_request_check_timed_out(struct fpm_child_s *child, struct timeval *tv,
int fpm_request_is_idle(struct fpm_child_s *child);
const char *fpm_request_get_stage_name(int stage);
int fpm_request_last_activity(struct fpm_child_s *child, struct timeval *tv);
+void fpm_request_accepted();
enum fpm_request_stage_e {
FPM_REQUEST_ACCEPTING = 1,
+ FPM_REQUEST_ACCEPTED,
FPM_REQUEST_READING_HEADERS,
FPM_REQUEST_INFO,
FPM_REQUEST_EXECUTING,
diff --git a/SOURCES/php/sapi/fpm/fpm/fpm_worker_pool.h b/SOURCES/php/sapi/fpm/fpm/fpm_worker_pool.h
index 6b2bc90..e1ecbe4 100644
--- a/SOURCES/php/sapi/fpm/fpm/fpm_worker_pool.h
+++ b/SOURCES/php/sapi/fpm/fpm/fpm_worker_pool.h
@@ -40,8 +40,10 @@ struct fpm_worker_pool_s {
char **limit_extensions;
/* for ondemand PM */
- struct fpm_event_s *ondemand_event;
+ struct fpm_event_s *ondemand_event, *child_accept_event;
int socket_event_set;
+ int child_accept_pipe[2];
+ int can_read_before_forking;
#ifdef HAVE_FPM_ACL
void *socket_acl;
|
Copyright © 2001-2024 The PHP Group