From 059f9f78e5f9256c4cf68c47bbda014c5e2bfe23 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 9 Oct 2025 18:58:10 +0200 Subject: [PATCH] Fix GH-19974: fpm_status_export_to_zval segfault for parallel execution The fix fixes some other races that could result in mixed up copy and nprocs number. It requires creating a copy in a similar way like for request status. This was not previously used to not impact other requests. However this does not make that much sense as the only thing that impacts it is holding the lock and not waiting for it. It is true that if there was a big contention then the lock would be acquired more often but that can be achieved by using fpm_get_status in loop so it should not make a huge difference hopefully. Closes GH-19974 --- NEWS | 4 ++ sapi/fpm/fpm/fpm_status.c | 101 ++++++++++++++++---------------------- 2 files changed, 46 insertions(+), 59 deletions(-) diff --git a/NEWS b/NEWS index 12ccb411bb519..b4b0521051f1d 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,10 @@ PHP NEWS . Fixed bug GH-20073 (Assertion failure in WeakMap offset operations on reference). (nielsdos) +- FPM: + . Fixed bug GH-19974 (fpm_status_export_to_zval segfault for parallel + execution). (Jakub Zelenka, txuna) + - Random: . Fix Randomizer::__serialize() w.r.t. INDIRECTs. (nielsdos) diff --git a/sapi/fpm/fpm/fpm_status.c b/sapi/fpm/fpm/fpm_status.c index f44274e9d9ee4..47fc0bac76d17 100644 --- a/sapi/fpm/fpm/fpm_status.c +++ b/sapi/fpm/fpm/fpm_status.c @@ -47,92 +47,75 @@ int fpm_status_init_child(struct fpm_worker_pool_s *wp) /* {{{ */ int fpm_status_export_to_zval(zval *status) { - struct fpm_scoreboard_s scoreboard, *scoreboard_p; + struct fpm_scoreboard_s *scoreboard_p; + struct fpm_scoreboard_proc_s *proc_p; zval fpm_proc_stats, fpm_proc_stat; time_t now_epoch; struct timeval duration, now; double cpu; int i; - scoreboard_p = fpm_scoreboard_acquire(NULL, 1); - if (!scoreboard_p) { - zlog(ZLOG_NOTICE, "[pool (unknown)] status: scoreboard already in use."); - return -1; - } - - /* copy the scoreboard not to bother other processes */ - scoreboard = *scoreboard_p; - struct fpm_scoreboard_proc_s *procs = safe_emalloc( - sizeof(struct fpm_scoreboard_proc_s), scoreboard.nprocs, 0); - - struct fpm_scoreboard_proc_s *proc_p; - for(i=0; ipool); + add_assoc_string(status, "process-manager", PM2STR(scoreboard_p->pm)); + add_assoc_long(status, "start-time", scoreboard_p->start_epoch); + add_assoc_long(status, "start-since", now_epoch - scoreboard_p->start_epoch); + add_assoc_long(status, "accepted-conn", scoreboard_p->requests); + add_assoc_long(status, "listen-queue", scoreboard_p->lq); + add_assoc_long(status, "max-listen-queue", scoreboard_p->lq_max); + add_assoc_long(status, "listen-queue-len", scoreboard_p->lq_len); + add_assoc_long(status, "idle-processes", scoreboard_p->idle); + add_assoc_long(status, "active-processes", scoreboard_p->active); + add_assoc_long(status, "total-processes", scoreboard_p->idle + scoreboard_p->active); + add_assoc_long(status, "max-active-processes", scoreboard_p->active_max); + add_assoc_long(status, "max-children-reached", scoreboard_p->max_children_reached); + add_assoc_long(status, "slow-requests", scoreboard_p->slow_rq); array_init(&fpm_proc_stats); - for(i=0; inprocs; i++) { + proc_p = &scoreboard_p->procs[i]; + if (!proc_p->used) { continue; } - proc_p = &procs[i]; /* prevent NaN */ - if (procs[i].cpu_duration.tv_sec == 0 && procs[i].cpu_duration.tv_usec == 0) { + if (proc_p->cpu_duration.tv_sec == 0 && proc_p->cpu_duration.tv_usec == 0) { cpu = 0.; } else { - cpu = (procs[i].last_request_cpu.tms_utime + procs[i].last_request_cpu.tms_stime + procs[i].last_request_cpu.tms_cutime + procs[i].last_request_cpu.tms_cstime) / fpm_scoreboard_get_tick() / (procs[i].cpu_duration.tv_sec + procs[i].cpu_duration.tv_usec / 1000000.) * 100.; + cpu = (proc_p->last_request_cpu.tms_utime + proc_p->last_request_cpu.tms_stime + proc_p->last_request_cpu.tms_cutime + + proc_p->last_request_cpu.tms_cstime) / fpm_scoreboard_get_tick() / + (proc_p->cpu_duration.tv_sec + proc_p->cpu_duration.tv_usec / 1000000.) * 100.; } array_init(&fpm_proc_stat); - add_assoc_long(&fpm_proc_stat, "pid", procs[i].pid); - add_assoc_string(&fpm_proc_stat, "state", fpm_request_get_stage_name(procs[i].request_stage)); - add_assoc_long(&fpm_proc_stat, "start-time", procs[i].start_epoch); - add_assoc_long(&fpm_proc_stat, "start-since", now_epoch - procs[i].start_epoch); - add_assoc_long(&fpm_proc_stat, "requests", procs[i].requests); - if (procs[i].request_stage == FPM_REQUEST_ACCEPTING) { - duration = procs[i].duration; + add_assoc_long(&fpm_proc_stat, "pid", proc_p->pid); + add_assoc_string(&fpm_proc_stat, "state", fpm_request_get_stage_name(proc_p->request_stage)); + add_assoc_long(&fpm_proc_stat, "start-time", proc_p->start_epoch); + add_assoc_long(&fpm_proc_stat, "start-since", now_epoch - proc_p->start_epoch); + add_assoc_long(&fpm_proc_stat, "requests", proc_p->requests); + if (proc_p->request_stage == FPM_REQUEST_ACCEPTING) { + duration = proc_p->duration; } else { - timersub(&now, &procs[i].accepted, &duration); + timersub(&now, &proc_p->accepted, &duration); } add_assoc_long(&fpm_proc_stat, "request-duration", duration.tv_sec * 1000000UL + duration.tv_usec); - add_assoc_string(&fpm_proc_stat, "request-method", procs[i].request_method[0] != '\0' ? procs[i].request_method : "-"); - add_assoc_string(&fpm_proc_stat, "request-uri", procs[i].request_uri); - add_assoc_string(&fpm_proc_stat, "query-string", procs[i].query_string); - add_assoc_long(&fpm_proc_stat, "request-length", procs[i].content_length); - add_assoc_string(&fpm_proc_stat, "user", procs[i].auth_user[0] != '\0' ? procs[i].auth_user : "-"); - add_assoc_string(&fpm_proc_stat, "script", procs[i].script_filename[0] != '\0' ? procs[i].script_filename : "-"); - add_assoc_double(&fpm_proc_stat, "last-request-cpu", procs[i].request_stage == FPM_REQUEST_ACCEPTING ? cpu : 0.); - add_assoc_long(&fpm_proc_stat, "last-request-memory", procs[i].request_stage == FPM_REQUEST_ACCEPTING ? procs[i].memory : 0); + add_assoc_string(&fpm_proc_stat, "request-method", proc_p->request_method[0] != '\0' ? proc_p->request_method : "-"); + add_assoc_string(&fpm_proc_stat, "request-uri", proc_p->request_uri); + add_assoc_string(&fpm_proc_stat, "query-string", proc_p->query_string); + add_assoc_long(&fpm_proc_stat, "request-length", proc_p->content_length); + add_assoc_string(&fpm_proc_stat, "user", proc_p->auth_user[0] != '\0' ? proc_p->auth_user : "-"); + add_assoc_string(&fpm_proc_stat, "script", proc_p->script_filename[0] != '\0' ? proc_p->script_filename : "-"); + add_assoc_double(&fpm_proc_stat, "last-request-cpu", proc_p->request_stage == FPM_REQUEST_ACCEPTING ? cpu : 0.); + add_assoc_long(&fpm_proc_stat, "last-request-memory", proc_p->request_stage == FPM_REQUEST_ACCEPTING ? proc_p->memory : 0); add_next_index_zval(&fpm_proc_stats, &fpm_proc_stat); } add_assoc_zval(status, "procs", &fpm_proc_stats); - efree(procs); + + fpm_scoreboard_free_copy(scoreboard_p); return 0; }