From 5db4105b7b2fc4a976841a286fef8812415f07f1 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sat, 9 Apr 2022 13:20:07 +0100 Subject: [PATCH] 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. --- sapi/fpm/fpm/fpm_process_ctl.c | 35 ++++++++++++++++++--------------- sapi/fpm/fpm/fpm_request.c | 8 ++++++-- sapi/fpm/fpm/fpm_scoreboard.c | 36 ++++++++++++++++++++++++++++++++-- sapi/fpm/fpm/fpm_scoreboard.h | 3 +++ 4 files changed, 62 insertions(+), 20 deletions(-) diff --git a/sapi/fpm/fpm/fpm_process_ctl.c b/sapi/fpm/fpm/fpm_process_ctl.c index a2f0f935e443c..4b8dca4690ccb 100644 --- a/sapi/fpm/fpm/fpm_process_ctl.c +++ b/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) { diff --git a/sapi/fpm/fpm/fpm_request.c b/sapi/fpm/fpm/fpm_request.c index 0a6f6a7cfbf0d..4f51824138d4d 100644 --- a/sapi/fpm/fpm/fpm_request.c +++ b/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); } /* }}} */ diff --git a/sapi/fpm/fpm/fpm_scoreboard.c b/sapi/fpm/fpm/fpm_scoreboard.c index ec28af49d7c4b..bf847d9dcac98 100644 --- a/sapi/fpm/fpm/fpm_scoreboard.c +++ b/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; diff --git a/sapi/fpm/fpm/fpm_scoreboard.h b/sapi/fpm/fpm/fpm_scoreboard.h index e22d6d933a00a..6925094936495 100644 --- a/sapi/fpm/fpm/fpm_scoreboard.h +++ b/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);