From a6f3a2b4e55ddbba33be0fb4706ecc534fdbfbcf Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Tue, 2 Dec 2025 22:28:35 +0100 Subject: [PATCH] Fix GH-18956: PHP-FPM Process Count Inconsistencies This fixes extra increments on keep alive that happen for follow up request in headers reading stage. It is because the accepting stage is only done on the first request. It adds a new flag to the on_read fastcgi callback that sets whether the kept alive request is being done and then skips the further active increments. It also fixes issue with incorrect decrement of the active number of processes. This was done in accepting stage even on the first which might still be problematic if there are already some running processes as it decrements their number first and result in incorrect total (lower than the actual number). The fix is to skip this decrement of active processes on the first accept. Closes GH-19191 --- main/fastcgi.c | 22 +++++++++++++++------- main/fastcgi.h | 2 +- sapi/fpm/fpm/fpm_request.c | 16 ++++++++++------ sapi/fpm/fpm/fpm_request.h | 4 ++-- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/main/fastcgi.c b/main/fastcgi.c index 60a30a5b66e6f..98b58977d8693 100644 --- a/main/fastcgi.c +++ b/main/fastcgi.c @@ -200,8 +200,8 @@ typedef struct _fcgi_hash { typedef struct _fcgi_req_hook fcgi_req_hook; struct _fcgi_req_hook { - void(*on_accept)(void); - void(*on_read)(void); + void(*on_accept)(bool firstAccept); + void(*on_read)(bool keptAlive); void(*on_close)(void); }; @@ -874,7 +874,11 @@ static void fcgi_hook_dummy(void) { return; } -fcgi_request *fcgi_init_request(int listen_socket, void(*on_accept)(void), void(*on_read)(void), void(*on_close)(void)) +static void fcgi_hook_dummy_bool(bool) { + return; +} + +fcgi_request *fcgi_init_request(int listen_socket, void(*on_accept)(bool), void(*on_read)(bool), void(*on_close)(void)) { fcgi_request *req = calloc(1, sizeof(fcgi_request)); req->listen_socket = listen_socket; @@ -896,8 +900,8 @@ fcgi_request *fcgi_init_request(int listen_socket, void(*on_accept)(void), void( */ req->out_pos = req->out_buf; - req->hook.on_accept = on_accept ? on_accept : fcgi_hook_dummy; - req->hook.on_read = on_read ? on_read : fcgi_hook_dummy; + req->hook.on_accept = on_accept ? on_accept : fcgi_hook_dummy_bool; + req->hook.on_read = on_read ? on_read : fcgi_hook_dummy_bool; req->hook.on_close = on_close ? on_close : fcgi_hook_dummy; #ifdef _WIN32 @@ -1364,6 +1368,8 @@ int fcgi_accept_request(fcgi_request *req) OVERLAPPED ov; #endif + bool keptAlive = false; + bool firstAccept = false; while (1) { if (req->fd < 0) { while (1) { @@ -1371,7 +1377,8 @@ int fcgi_accept_request(fcgi_request *req) return -1; } - req->hook.on_accept(); + req->hook.on_accept(firstAccept); + firstAccept = keptAlive = false; #ifdef _WIN32 if (!req->tcp) { pipe = (HANDLE)_get_osfhandle(req->listen_socket); @@ -1479,7 +1486,8 @@ int fcgi_accept_request(fcgi_request *req) } else if (in_shutdown) { return -1; } - req->hook.on_read(); + req->hook.on_read(keptAlive); + keptAlive = true; int read_result = fcgi_read_request(req); if (read_result == 1) { #ifdef _WIN32 diff --git a/main/fastcgi.h b/main/fastcgi.h index 8a1c0516a9c7a..a0b73121637c2 100644 --- a/main/fastcgi.h +++ b/main/fastcgi.h @@ -91,7 +91,7 @@ void fcgi_close(fcgi_request *req, int force, int destroy); int fcgi_in_shutdown(void); void fcgi_terminate(void); int fcgi_listen(const char *path, int backlog); -fcgi_request* fcgi_init_request(int listen_socket, void(*on_accept)(void), void(*on_read)(void), void(*on_close)(void)); +fcgi_request* fcgi_init_request(int listen_socket, void(*on_accept)(bool), void(*on_read)(bool), void(*on_close)(void)); void fcgi_destroy_request(fcgi_request *req); void fcgi_set_allowed_clients(char *ip); int fcgi_accept_request(fcgi_request *req); diff --git a/sapi/fpm/fpm/fpm_request.c b/sapi/fpm/fpm/fpm_request.c index 9ea7d8aeaaa35..ef0da09145f43 100644 --- a/sapi/fpm/fpm/fpm_request.c +++ b/sapi/fpm/fpm/fpm_request.c @@ -35,7 +35,7 @@ const char *fpm_request_get_stage_name(int stage) { return requests_stages[stage]; } -void fpm_request_accepting(void) +void fpm_request_accepting(bool firstAccept) { struct fpm_scoreboard_proc_s *proc; struct timeval now; @@ -54,11 +54,13 @@ void fpm_request_accepting(void) proc->tv = now; fpm_scoreboard_proc_release(proc); - /* idle++, active-- */ - fpm_scoreboard_update_commit(1, -1, 0, 0, 0, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL); + if (!firstAccept) { + /* idle++, active-- */ + fpm_scoreboard_update_commit(1, -1, 0, 0, 0, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL); + } } -void fpm_request_reading_headers(void) +void fpm_request_reading_headers(bool keptAlive) { struct fpm_scoreboard_proc_s *proc; @@ -98,8 +100,10 @@ void fpm_request_reading_headers(void) proc->content_length = 0; fpm_scoreboard_proc_release(proc); - /* idle--, active++, request++ */ - fpm_scoreboard_update_commit(-1, 1, 0, 0, 1, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL); + if (!keptAlive) { + /* idle--, active++, request++ */ + fpm_scoreboard_update_commit(-1, 1, 0, 0, 1, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL); + } } void fpm_request_info(void) diff --git a/sapi/fpm/fpm/fpm_request.h b/sapi/fpm/fpm/fpm_request.h index 1dcc7f78902fc..787b6d1696497 100644 --- a/sapi/fpm/fpm/fpm_request.h +++ b/sapi/fpm/fpm/fpm_request.h @@ -4,9 +4,9 @@ #define FPM_REQUEST_H 1 /* hanging in accept() */ -void fpm_request_accepting(void); +void fpm_request_accepting(bool fistAccept); /* start reading fastcgi request from very first byte */ -void fpm_request_reading_headers(void); +void fpm_request_reading_headers(bool keptAlive); /* not a stage really but a point in the php code, where all request params have become known to sapi */ void fpm_request_info(void); /* the script is executing */