Skip to content

Commit a6f3a2b

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 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
1 parent 8fe7930 commit a6f3a2b

File tree

4 files changed

+28
-16
lines changed

4 files changed

+28
-16
lines changed

main/fastcgi.c

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,8 @@ typedef struct _fcgi_hash {
200200
typedef struct _fcgi_req_hook fcgi_req_hook;
201201

202202
struct _fcgi_req_hook {
203-
void(*on_accept)(void);
204-
void(*on_read)(void);
203+
void(*on_accept)(bool firstAccept);
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)(bool), void(*on_read)(bool), void(*on_close)(void))
878882
{
879883
fcgi_request *req = calloc(1, sizeof(fcgi_request));
880884
req->listen_socket = listen_socket;
@@ -896,8 +900,8 @@ fcgi_request *fcgi_init_request(int listen_socket, void(*on_accept)(void), void(
896900
897901
*/
898902
req->out_pos = req->out_buf;
899-
req->hook.on_accept = on_accept ? on_accept : fcgi_hook_dummy;
900-
req->hook.on_read = on_read ? on_read : fcgi_hook_dummy;
903+
req->hook.on_accept = on_accept ? on_accept : fcgi_hook_dummy_bool;
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,14 +1368,17 @@ int fcgi_accept_request(fcgi_request *req)
13641368
OVERLAPPED ov;
13651369
#endif
13661370

1371+
bool keptAlive = false;
1372+
bool firstAccept = false;
13671373
while (1) {
13681374
if (req->fd < 0) {
13691375
while (1) {
13701376
if (in_shutdown) {
13711377
return -1;
13721378
}
13731379

1374-
req->hook.on_accept();
1380+
req->hook.on_accept(firstAccept);
1381+
firstAccept = keptAlive = false;
13751382
#ifdef _WIN32
13761383
if (!req->tcp) {
13771384
pipe = (HANDLE)_get_osfhandle(req->listen_socket);
@@ -1479,7 +1486,8 @@ int fcgi_accept_request(fcgi_request *req)
14791486
} else if (in_shutdown) {
14801487
return -1;
14811488
}
1482-
req->hook.on_read();
1489+
req->hook.on_read(keptAlive);
1490+
keptAlive = true;
14831491
int read_result = fcgi_read_request(req);
14841492
if (read_result == 1) {
14851493
#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)(bool), 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: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const char *fpm_request_get_stage_name(int stage) {
3535
return requests_stages[stage];
3636
}
3737

38-
void fpm_request_accepting(void)
38+
void fpm_request_accepting(bool firstAccept)
3939
{
4040
struct fpm_scoreboard_proc_s *proc;
4141
struct timeval now;
@@ -54,11 +54,13 @@ void fpm_request_accepting(void)
5454
proc->tv = now;
5555
fpm_scoreboard_proc_release(proc);
5656

57-
/* idle++, active-- */
58-
fpm_scoreboard_update_commit(1, -1, 0, 0, 0, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL);
57+
if (!firstAccept) {
58+
/* idle++, active-- */
59+
fpm_scoreboard_update_commit(1, -1, 0, 0, 0, 0, 0, FPM_SCOREBOARD_ACTION_INC, NULL);
60+
}
5961
}
6062

61-
void fpm_request_reading_headers(void)
63+
void fpm_request_reading_headers(bool keptAlive)
6264
{
6365
struct fpm_scoreboard_proc_s *proc;
6466

@@ -98,8 +100,10 @@ void fpm_request_reading_headers(void)
98100
proc->content_length = 0;
99101
fpm_scoreboard_proc_release(proc);
100102

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

105109
void fpm_request_info(void)

sapi/fpm/fpm/fpm_request.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
#define FPM_REQUEST_H 1
55

66
/* hanging in accept() */
7-
void fpm_request_accepting(void);
7+
void fpm_request_accepting(bool fistAccept);
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)