php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Return to Bug #69724
Patch SmallerPatchKeepsEdgeLevelAndChildrenTellParentsMore revision 2016-11-24 16:47 UTC by nick at cpanel dot net
Patch Updatedpatchforlatest5.6 revision 2016-08-24 23:19 UTC by nick at cpanel dot net
Patch ywaqvokc revision 2015-07-16 12:07 UTC by sample at email dot tst
revision 2015-07-16 12:07 UTC by sample at email dot tst
revision 2015-07-16 12:07 UTC by sample at email dot tst
revision 2015-07-16 12:06 UTC by sample at email dot tst
Patch blqcykmi revision 2015-07-16 12:01 UTC by sample at email dot tst

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;
 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Mon Jun 21 01:01:23 2021 UTC