Skip to content

Commit 3586d5e

Browse files
committed
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 might still leave issues with incorrect decrement of the active number of processes. This is done in accepting stage before incrementing 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). Closes GH-19191
1 parent 8fe7930 commit 3586d5e

File tree

4 files changed

+18
-9
lines changed

4 files changed

+18
-9
lines changed

main/fastcgi.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ typedef struct _fcgi_req_hook fcgi_req_hook;
201201

202202
struct _fcgi_req_hook {
203203
void(*on_accept)(void);
204-
void(*on_read)(void);
204+
void(*on_read)(bool keptAlive);
205205
void(*on_close)(void);
206206
};
207207

@@ -874,7 +874,11 @@ static void fcgi_hook_dummy(void) {
874874
return;
875875
}
876876

877-
fcgi_request *fcgi_init_request(int listen_socket, void(*on_accept)(void), void(*on_read)(void), void(*on_close)(void))
877+
static void fcgi_hook_dummy_bool(bool) {
878+
return;
879+
}
880+
881+
fcgi_request *fcgi_init_request(int listen_socket, void(*on_accept)(void), void(*on_read)(bool), void(*on_close)(void))
878882
{
879883
fcgi_request *req = calloc(1, sizeof(fcgi_request));
880884
req->listen_socket = listen_socket;
@@ -897,7 +901,7 @@ fcgi_request *fcgi_init_request(int listen_socket, void(*on_accept)(void), void(
897901
*/
898902
req->out_pos = req->out_buf;
899903
req->hook.on_accept = on_accept ? on_accept : fcgi_hook_dummy;
900-
req->hook.on_read = on_read ? on_read : fcgi_hook_dummy;
904+
req->hook.on_read = on_read ? on_read : fcgi_hook_dummy_bool;
901905
req->hook.on_close = on_close ? on_close : fcgi_hook_dummy;
902906

903907
#ifdef _WIN32
@@ -1364,6 +1368,7 @@ int fcgi_accept_request(fcgi_request *req)
13641368
OVERLAPPED ov;
13651369
#endif
13661370

1371+
bool keptAlive = false;
13671372
while (1) {
13681373
if (req->fd < 0) {
13691374
while (1) {
@@ -1372,6 +1377,7 @@ int fcgi_accept_request(fcgi_request *req)
13721377
}
13731378

13741379
req->hook.on_accept();
1380+
keptAlive = false;
13751381
#ifdef _WIN32
13761382
if (!req->tcp) {
13771383
pipe = (HANDLE)_get_osfhandle(req->listen_socket);
@@ -1479,7 +1485,8 @@ int fcgi_accept_request(fcgi_request *req)
14791485
} else if (in_shutdown) {
14801486
return -1;
14811487
}
1482-
req->hook.on_read();
1488+
req->hook.on_read(keptAlive);
1489+
keptAlive = true;
14831490
int read_result = fcgi_read_request(req);
14841491
if (read_result == 1) {
14851492
#ifdef _WIN32

main/fastcgi.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ void fcgi_close(fcgi_request *req, int force, int destroy);
9191
int fcgi_in_shutdown(void);
9292
void fcgi_terminate(void);
9393
int fcgi_listen(const char *path, int backlog);
94-
fcgi_request* fcgi_init_request(int listen_socket, void(*on_accept)(void), void(*on_read)(void), void(*on_close)(void));
94+
fcgi_request* fcgi_init_request(int listen_socket, void(*on_accept)(void), void(*on_read)(bool), void(*on_close)(void));
9595
void fcgi_destroy_request(fcgi_request *req);
9696
void fcgi_set_allowed_clients(char *ip);
9797
int fcgi_accept_request(fcgi_request *req);

sapi/fpm/fpm/fpm_request.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ void fpm_request_accepting(void)
5858
fpm_scoreboard_update_commit(1, -1, 0, 0, 0, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL);
5959
}
6060

61-
void fpm_request_reading_headers(void)
61+
void fpm_request_reading_headers(bool keptAlive)
6262
{
6363
struct fpm_scoreboard_proc_s *proc;
6464

@@ -98,8 +98,10 @@ void fpm_request_reading_headers(void)
9898
proc->content_length = 0;
9999
fpm_scoreboard_proc_release(proc);
100100

101-
/* idle--, active++, request++ */
102-
fpm_scoreboard_update_commit(-1, 1, 0, 0, 1, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL);
101+
if (!keptAlive) {
102+
/* idle--, active++, request++ */
103+
fpm_scoreboard_update_commit(-1, 1, 0, 0, 1, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL);
104+
}
103105
}
104106

105107
void fpm_request_info(void)

sapi/fpm/fpm/fpm_request.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
/* hanging in accept() */
77
void fpm_request_accepting(void);
88
/* start reading fastcgi request from very first byte */
9-
void fpm_request_reading_headers(void);
9+
void fpm_request_reading_headers(bool keptAlive);
1010
/* not a stage really but a point in the php code, where all request params have become known to sapi */
1111
void fpm_request_info(void);
1212
/* the script is executing */

0 commit comments

Comments
 (0)