Fix bug #76003: FPM /status reports wrong number of active processe

The fix introduces early locking of scoreboard when it is updated
which prevents the race condition causing an incorrect number of
active processes being set.

Author: bukka

NEWS

@@ -6,6 +6,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-8289 (Exceptions thrown within a yielded from iterator are
     not rethrown into the generator). (Bob)
 
+- FPM:
+  . Fixed bug #76003 (FPM /status reports wrong number of active processe).
+    (Jakub Zelenka)
+
 - MySQLi:
   . Fixed bug GH-8267 (MySQLi uses unsupported format specifier on Windows).
     (cmb)

sapi/fpm/fpm/fpm_process_ctl.c

@@ -324,21 +324,6 @@ static void fpm_pctl_perform_idle_server_maintenance(struct timeval *now) /* {{{
 
 		if (wp->config == NULL) continue;
 
-		for (child = wp->children; child; child = child->next) {
-			if (fpm_request_is_idle(child)) {
-				if (last_idle_child == NULL) {
-					last_idle_child = child;
-				} else {
-					if (timercmp(&child->started, &last_idle_child->started, <)) {
-						last_idle_child = child;
-					}
-				}
-				idle++;
-			} else {
-				active++;
-			}
-		}
-
 		/* update status structure for all PMs */
 		if (wp->listen_address_domain == FPM_AF_INET) {
 			if (0 > fpm_socket_get_listening_queue(wp->listening_socket, &cur_lq, NULL)) {
@@ -356,7 +341,25 @@ static void fpm_pctl_perform_idle_server_maintenance(struct timeval *now) /* {{{
 #endif
 			}
 		}
-		fpm_scoreboard_update(idle, active, cur_lq, -1, -1, -1, 0, FPM_SCOREBOARD_ACTION_SET, wp->scoreboard);
+
+		fpm_scoreboard_update_begin(wp->scoreboard);
+
+		for (child = wp->children; child; child = child->next) {
+			if (fpm_request_is_idle(child)) {
+				if (last_idle_child == NULL) {
+					last_idle_child = child;
+				} else {
+					if (timercmp(&child->started, &last_idle_child->started, <)) {
+						last_idle_child = child;
+					}
+				}
+				idle++;
+			} else {
+				active++;
+			}
+		}
+
+		fpm_scoreboard_update_commit(idle, active, cur_lq, -1, -1, -1, 0, FPM_SCOREBOARD_ACTION_SET, wp->scoreboard);
 
 		/* this is specific to PM_STYLE_ONDEMAND */
 		if (wp->config->pm == PM_STYLE_ONDEMAND) {

sapi/fpm/fpm/fpm_request.c

@@ -41,6 +41,8 @@ void fpm_request_accepting() /* {{{ */
 
 	fpm_clock_get(&now);
 
+	fpm_scoreboard_update_begin(NULL);
+
 	proc = fpm_scoreboard_proc_acquire(NULL, -1, 0);
 	if (proc == NULL) {
 		zlog(ZLOG_WARNING, "failed to acquire proc scoreboard");
@@ -52,7 +54,7 @@ void fpm_request_accepting() /* {{{ */
 	fpm_scoreboard_proc_release(proc);
 
 	/* idle++, active-- */
-	fpm_scoreboard_update(1, -1, 0, 0, 0, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL);
+	fpm_scoreboard_update_commit(1, -1, 0, 0, 0, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL);
 }
 /* }}} */
 
@@ -72,6 +74,8 @@ void fpm_request_reading_headers() /* {{{ */
 	times(&cpu);
 #endif
 
+	fpm_scoreboard_update_begin(NULL);
+
 	proc = fpm_scoreboard_proc_acquire(NULL, -1, 0);
 	if (proc == NULL) {
 		zlog(ZLOG_WARNING, "failed to acquire proc scoreboard");
@@ -95,7 +99,7 @@ void fpm_request_reading_headers() /* {{{ */
 	fpm_scoreboard_proc_release(proc);
 
 	/* idle--, active++, request++ */
-	fpm_scoreboard_update(-1, 1, 0, 0, 1, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL);
+	fpm_scoreboard_update_commit(-1, 1, 0, 0, 1, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL);
 }
 /* }}} */
 

sapi/fpm/fpm/fpm_scoreboard.c

@@ -74,18 +74,39 @@ int fpm_scoreboard_init_main() /* {{{ */
 }
 /* }}} */
 
-void fpm_scoreboard_update(int idle, int active, int lq, int lq_len, int requests, int max_children_reached, int slow_rq, int action, struct fpm_scoreboard_s *scoreboard) /* {{{ */
+static struct fpm_scoreboard_s *fpm_scoreboard_get_for_update(struct fpm_scoreboard_s *scoreboard) /* {{{ */
 {
 	if (!scoreboard) {
 		scoreboard = fpm_scoreboard;
 	}
 	if (!scoreboard) {
 		zlog(ZLOG_WARNING, "Unable to update scoreboard: the SHM has not been found");
-		return;
 	}
 
+	return scoreboard;
+}
+/* }}} */
+
+void fpm_scoreboard_update_begin(struct fpm_scoreboard_s *scoreboard) /* {{{ */
+{
+	scoreboard = fpm_scoreboard_get_for_update(scoreboard);
+	if (!scoreboard) {
+		return;
+	}
 
 	fpm_spinlock(&scoreboard->lock, 0);
+}
+/* }}} */
+
+void fpm_scoreboard_update_commit(
+		int idle, int active, int lq, int lq_len, int requests, int max_children_reached,
+		int slow_rq, int action, struct fpm_scoreboard_s *scoreboard) /* {{{ */
+{
+	scoreboard = fpm_scoreboard_get_for_update(scoreboard);
+	if (!scoreboard) {
+		return;
+	}
+
 	if (action == FPM_SCOREBOARD_ACTION_SET) {
 		if (idle >= 0) {
 			scoreboard->idle = idle;
@@ -154,6 +175,17 @@ void fpm_scoreboard_update(int idle, int active, int lq, int lq_len, int request
 }
 /* }}} */
 
+
+void fpm_scoreboard_update(
+		int idle, int active, int lq, int lq_len, int requests, int max_children_reached,
+		int slow_rq, int action, struct fpm_scoreboard_s *scoreboard) /* {{{ */
+{
+	fpm_scoreboard_update_begin(scoreboard);
+	fpm_scoreboard_update_commit(
+			idle, active, lq, lq_len, requests, max_children_reached, slow_rq, action, scoreboard);
+}
+/* }}} */
+
 struct fpm_scoreboard_s *fpm_scoreboard_get() /* {{{*/
 {
 	return fpm_scoreboard;

sapi/fpm/fpm/fpm_scoreboard.h

@@ -73,7 +73,10 @@ struct fpm_scoreboard_s {
 int fpm_scoreboard_init_main();
 int fpm_scoreboard_init_child(struct fpm_worker_pool_s *wp);
 
+void fpm_scoreboard_update_begin(struct fpm_scoreboard_s *scoreboard);
+void fpm_scoreboard_update_commit(int idle, int active, int lq, int lq_len, int requests, int max_children_reached, int slow_rq, int action, struct fpm_scoreboard_s *scoreboard);
 void fpm_scoreboard_update(int idle, int active, int lq, int lq_len, int requests, int max_children_reached, int slow_rq, int action, struct fpm_scoreboard_s *scoreboard);
+
 struct fpm_scoreboard_s *fpm_scoreboard_get();
 struct fpm_scoreboard_proc_s *fpm_scoreboard_proc_get(struct fpm_scoreboard_s *scoreboard, int child_index);
 struct fpm_scoreboard_proc_s *fpm_scoreboard_proc_get_from_child(struct fpm_child_s *child);