apache_finish_request() alternative to fastcgi_finish_request()

[divinity76]

Description

would be nice if the apache / apache2handler sapi got a apache_finish_request() like the fastcgi_finish_request() function. quoting the fastcgi_finish_request() documentation:

This function flushes all response data to the client and finishes the request. This allows for time consuming tasks to be performed without leaving the connection to the client open.

[nielsdos]

I looked into this, but the way that it currently works is with synchronous handling of requests within a worker. I didn't find an Apache API that would let us do this. And hacking it by forcefully aborting the connection only half-works because the connection will be aborted by interrupting the TCP socket, which isn't ideal.

[divinity76]

seems @rlerdorf (the founder of PHP) looked into this problem himself, and used eos_bucket=apr_bucket_eos_create() + APR_BRIGADE_INSERT_TAIL(eos_bucket) to solve it: https://gist.github.com/rlerdorf/04cc9f2ecb88baa226e00bd612fb63b0

@nielsdos any chance apr_bucket_eos_create() is the API you were looking for?

[nielsdos]

Thanks for the link, I'll look into it some time soon.

[rlerdorf]

This was not possible to do because of a bug in Apache that @ericnorris fixed recently. That fix has been merged into Apache now. See https://lists.apache.org/thread/vbgwjdt5xpgjzpbx8blbmmzx73m1m816 for context. My apache_finish_request() will work with versions of Apache that has this fix.

[rlerdorf]

Here is what we are using in production:

diff --git a/sapi/apache2handler/php_apache.h b/sapi/apache2handler/php_apache.h
index 19c10e23ce..8bcd547d82 100644
--- a/sapi/apache2handler/php_apache.h
+++ b/sapi/apache2handler/php_apache.h
@@ -47,6 +47,7 @@ typedef struct php_struct {
        int request_processed;
        /* final content type */
        char *content_type;
+       int eos_sent;
 } php_struct;

 void *merge_php_config(apr_pool_t *p, void *base_conf, void *new_conf);
@@ -55,6 +56,7 @@ char *get_php_config(void *conf, char *name, size_t name_len);
 void apply_config(void *);
 extern const command_rec php_dir_cmds[];
 void php_ap2_register_hook(apr_pool_t *p);
+zend_bool php_ap2_finish_request();

 #define APR_ARRAY_FOREACH_OPEN(arr, key, val)          \
 {                                                                                                      \
diff --git a/sapi/apache2handler/php_functions.c b/sapi/apache2handler/php_functions.c
index 27b8aab1df..80b89553df 100644
--- a/sapi/apache2handler/php_functions.c
+++ b/sapi/apache2handler/php_functions.c
@@ -26,6 +26,7 @@
 #include "zend_smart_str.h"
 #include "ext/standard/info.h"
 #include "ext/standard/head.h"
+#include "php_main.h"
 #include "php_ini.h"
 #include "SAPI.h"

@@ -358,6 +359,24 @@ PHP_FUNCTION(apache_get_modules)
 }
 /* }}} */

+/* {{{ Force Apache request to finish */
+PHP_FUNCTION(apache_finish_request)
+{
+       if (zend_parse_parameters_none() == FAILURE) {
+               RETURN_THROWS();
+       }
+
+       php_output_end_all();
+       php_output_deactivate();
+
+       if(php_ap2_finish_request()) {
+               RETURN_TRUE;
+       } else {
+               RETURN_FALSE;
+       }
+}
+/* }}} */
+
 PHP_MINFO_FUNCTION(apache)
 {
        char *apv = php_apache_get_version();
diff --git a/sapi/apache2handler/php_functions.stub.php b/sapi/apache2handler/php_functions.stub.php
index 99a401dbab..c243ffd4bd 100644
--- a/sapi/apache2handler/php_functions.stub.php
+++ b/sapi/apache2handler/php_functions.stub.php
@@ -22,3 +22,5 @@ function apache_getenv(string $variable, bool $walk_to_top = false): string|fals
 function apache_get_version(): string|false {}

 function apache_get_modules(): array {}
+
+function apache_finish_request(): bool {}
diff --git a/sapi/apache2handler/php_functions_arginfo.h b/sapi/apache2handler/php_functions_arginfo.h
index 0830189057..854f0bf45d 100644
Binary files a/sapi/apache2handler/php_functions_arginfo.h and b/sapi/apache2handler/php_functions_arginfo.h differ
diff --git a/sapi/apache2handler/sapi_apache2.c b/sapi/apache2handler/sapi_apache2.c
index a5d4e8984a..5ae2cdba37 100644
--- a/sapi/apache2handler/sapi_apache2.c
+++ b/sapi/apache2handler/sapi_apache2.c
@@ -301,8 +301,6 @@ php_apache_sapi_flush(void *server_context)
        r = ctx->r;

        sapi_send_headers();
-
-       r->status = SG(sapi_headers).http_response_code;
        SG(headers_sent) = 1;

        if (ap_rflush(r) < 0 || r->connection->aborted) {
@@ -732,18 +730,22 @@ zend_first_try {

        if (!parent_req) {
                php_apache_request_dtor(r);
-               ctx->request_processed = 1;
-               apr_brigade_cleanup(brigade);
-               bucket = apr_bucket_eos_create(r->connection->bucket_alloc);
-               APR_BRIGADE_INSERT_TAIL(brigade, bucket);
-
-               rv = ap_pass_brigade(r->output_filters, brigade);
-               if (rv != APR_SUCCESS || r->connection->aborted) {
+               if (!ctx->eos_sent) {
+                       ctx->request_processed = 1;
+                       apr_brigade_cleanup(brigade);
+                       bucket = apr_bucket_eos_create(r->connection->bucket_alloc);
+                       APR_BRIGADE_INSERT_TAIL(brigade, bucket);
+
+                       rv = ap_pass_brigade(r->output_filters, brigade);
+                       if (rv != APR_SUCCESS || r->connection->aborted) {
 zend_first_try {
-                       php_handle_aborted_connection();
+                               php_handle_aborted_connection();
 } zend_end_try();
+                       } else {
+                               ctx->eos_sent = 1;
+                       }
+                       apr_brigade_cleanup(brigade);
                }
-               apr_brigade_cleanup(brigade);
                apr_pool_cleanup_run(r->pool, (void *)&SG(server_context), php_server_context_cleanup);
        } else {
                ctx->r = parent_req;
@@ -764,6 +766,49 @@ static void php_apache_signal_init(apr_pool_t *pchild, server_rec *s)
 }
 #endif

+zend_bool php_ap2_finish_request()
+{
+       php_struct * volatile ctx;
+       request_rec * volatile r;
+       apr_bucket_brigade * volatile brigade;
+       apr_bucket *bucket;
+       apr_status_t rv;
+       zend_bool ret = 0;
+
+       ctx = SG(server_context);
+       if (!ctx) {
+               return ret;
+       }
+
+       sapi_send_headers();
+       SG(headers_sent) = 1;
+
+       r = ctx->r;
+       brigade = ctx->brigade;
+
+    // Insert a flush bucket to flush out the above content
+    bucket = apr_bucket_flush_create(r->connection->bucket_alloc);
+    APR_BRIGADE_INSERT_TAIL(brigade, bucket);
+    // Insert an end of stream bucket to signal that no more content will be coming
+    bucket = apr_bucket_eos_create(r->connection->bucket_alloc);
+    APR_BRIGADE_INSERT_TAIL(brigade, bucket);
+
+       rv = ap_pass_brigade(r->output_filters, brigade);
+
+       if (rv != APR_SUCCESS || r->connection->aborted) {
+               zend_first_try {
+                       php_handle_aborted_connection();
+               } zend_end_try();
+       } else {
+               ret = 1;
+       }
+
+       apr_brigade_cleanup(brigade);
+       ctx->eos_sent = 1;
+
+       return ret;
+}
+
 void php_ap2_register_hook(apr_pool_t *p)
 {
        ap_hook_pre_config(php_pre_config, NULL, NULL, APR_HOOK_MIDDLE);

[nielsdos]

Thanks for sharing that. It seems like a correct approach and the fact you have it running in production also further proves that. Is that something you'd like to PR? I think only codestyle nits would need to be addressed, e.g. the use of bool instead of zend_bool nowadays and use a bool for eos instead of int.

[Vitch612]

It would be great if this could make it to core php. While it made sense when PHP was a simple web scripting engine to tie its execution to flow to a web request, it is now a limitation which should be overcome. There are so many cases where some post-processing is needed and in many cases flushing the buffer is not enough for the requesting application to consider the request over (mobile or desktop apps mainly but also browsers who consider the request still pending as long as the script hasn't terminated its execution). The hacks that allow achieving this (cron like scripts) are not good enough for some cases where post processing is logically linked to each request, saving enough context for a cron to handle it later is a huge overhead. Also not strictly related to this thread but in the same spirit, seeing parallel make it to core would be a much better improvement to PHP than the work that has been (and is being) done on PHP 8.