From 87a544007d8f50eda04e64e2fb50ef85a3004295 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sun, 26 Jun 2022 13:40:14 +0100 Subject: [PATCH] Separate SAPI deactivating and destroying resources Currently some SAPI might bail out on deactivation. One of those SAPI is PHP-FPM that can bail out on request end if for example the connection is closed by the client (web sever). The problem is that in such case the resources are not freed and some values reset. The most visible impact can have not resetting the PG(headers_sent) which can cause erorrs in the next request. One such issue is described in #77780 bug. It seems reasonable to separate deactivation and destroying the resource which means that the bail out will not impact it. --- main/SAPI.c | 4 ++++ main/SAPI.h | 1 + main/main.c | 16 +++++++++++----- sapi/cli/php_cli.c | 2 ++ sapi/phpdbg/phpdbg.c | 1 + 5 files changed, 19 insertions(+), 5 deletions(-) diff --git a/main/SAPI.c b/main/SAPI.c index 70ed6a2b9c835..a341746e0c2cc 100644 --- a/main/SAPI.c +++ b/main/SAPI.c @@ -523,6 +523,10 @@ SAPI_API void sapi_deactivate(void) if (sapi_module.deactivate) { sapi_module.deactivate(); } +} + +SAPI_API void sapi_destroy(void) +{ if (SG(rfc1867_uploaded_files)) { destroy_uploaded_files_hash(); } diff --git a/main/SAPI.h b/main/SAPI.h index 77ab84806fe97..ab70a31382cdc 100644 --- a/main/SAPI.h +++ b/main/SAPI.h @@ -144,6 +144,7 @@ SAPI_API void sapi_startup(sapi_module_struct *sf); SAPI_API void sapi_shutdown(void); SAPI_API void sapi_activate(void); SAPI_API void sapi_deactivate(void); +SAPI_API void sapi_destroy(void); SAPI_API void sapi_initialize_empty_request(void); SAPI_API void sapi_add_request_header(const char *var, unsigned int var_len, char *val, unsigned int val_len, void *arg); END_EXTERN_C() diff --git a/main/main.c b/main/main.c index cccf694550fe3..17c7c81c62aba 100644 --- a/main/main.c +++ b/main/main.c @@ -1853,20 +1853,25 @@ void php_request_shutdown(void *dummy) zend_post_deactivate_modules(); } zend_end_try(); - /* 12. SAPI related shutdown (free stuff) */ + /* 12. SAPI related shutdown */ zend_try { sapi_deactivate(); } zend_end_try(); - /* 13. free virtual CWD memory */ + /* 13. SAPI destroying resources */ + zend_try { + sapi_destroy(); + } zend_end_try(); + + /* 14. free virtual CWD memory */ virtual_cwd_deactivate(); - /* 14. Destroy stream hashes */ + /* 15. Destroy stream hashes */ zend_try { php_shutdown_stream_hashes(); } zend_end_try(); - /* 15. Free Willy (here be crashes) */ + /* 16. Free Willy (here be crashes) */ zend_arena_destroy(CG(arena)); zend_interned_strings_deactivate(); zend_try { @@ -1877,7 +1882,7 @@ void php_request_shutdown(void *dummy) * At this point, no memory beyond a single chunk should be in use. */ zend_set_memory_limit(PG(memory_limit)); - /* 16. Deactivate Zend signals */ + /* 17. Deactivate Zend signals */ #ifdef ZEND_SIGNALS zend_signal_deactivate(); #endif @@ -2343,6 +2348,7 @@ zend_result php_module_startup(sapi_module_struct *sf, zend_module_entry *additi virtual_cwd_deactivate(); sapi_deactivate(); + sapi_destroy(); module_startup = false; /* Don't leak errors from startup into the per-request phase. */ diff --git a/sapi/cli/php_cli.c b/sapi/cli/php_cli.c index 625a6534b75f9..151af8cc1d267 100644 --- a/sapi/cli/php_cli.c +++ b/sapi/cli/php_cli.c @@ -660,6 +660,7 @@ static int do_cli(int argc, char **argv) /* {{{ */ get_zend_version() ); sapi_deactivate(); + sapi_destroy(); goto out; case 'm': /* list compiled in modules */ @@ -1147,6 +1148,7 @@ static int do_cli(int argc, char **argv) /* {{{ */ return EG(exit_status); err: sapi_deactivate(); + sapi_destroy(); zend_ini_deactivate(); EG(exit_status) = 1; goto out; diff --git a/sapi/phpdbg/phpdbg.c b/sapi/phpdbg/phpdbg.c index 440f526f2bed5..7e2c9cb91bab6 100644 --- a/sapi/phpdbg/phpdbg.c +++ b/sapi/phpdbg/phpdbg.c @@ -1383,6 +1383,7 @@ int main(int argc, char **argv) /* {{{ */ ); } sapi_deactivate(); + sapi_destroy(); sapi_shutdown(); php_ini_builder_deinit(&ini_builder); if (ini_override) {