Redesign of request cleanup and logging to use End-Of-Request bucket

(backport from async-dev branch to 2.3 trunk)


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@327925 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 140 insertions, 22 deletions

diff --git a/CHANGES b/CHANGES
index 5196cb1ebe..942a8e5a9b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,12 @@
 Changes with Apache 2.3.0
   [Remove entries to the current 2.0 and 2.2 section below, when backported]
 
+  *) Added an End-Of-Request bucket type.  The logging of a request and
+     the freeing of its pool are now done when the EOR bucket is destroyed.
+     This has the effect of delaying the logging until right after the last
+     of the response is sent; ap_core_output_filter() calls the access logger
+     indirectly when it destroys the EOR bucket.  [Brian Pane]
+
   *) Rewrite of logresolve support utility: IPv6 addresses are now supported
      and the format of statistical output has changed. [Colm MacCarthaigh]
 
diff --git a/include/http_request.h b/include/http_request.h
index 240559cea1..c76219bb43 100644
--- a/include/http_request.h
+++ b/include/http_request.h
@@ -237,12 +237,21 @@ AP_DECLARE(void) ap_allow_standard_methods(request_rec *r, int reset, ...);
 
 #ifdef CORE_PRIVATE
 /**
- * Process a top-level request from a client
+ * Process a top-level request from a client, and synchronously write
+ * the response to the client
  * @param r The current request
  */
 void ap_process_request(request_rec *);
 
 /**
+ * Process a top-level request from a client, allowing some or all of
+ * the response to remain buffered in the core output filter for later,
+ * asynchronous write completion
+ * @param r The current request
+ */
+void ap_process_async_request(request_rec *);
+
+/**
  * Kill the current request
  * @param type Why the request is dieing
  * @param r The current request
@@ -354,6 +363,36 @@ AP_DECLARE(int) ap_location_walk(request_rec *r);
 AP_DECLARE(int) ap_directory_walk(request_rec *r);
 AP_DECLARE(int) ap_file_walk(request_rec *r);
 
+/** End Of REQUEST (EOR) bucket */
+AP_DECLARE_DATA extern const apr_bucket_type_t ap_bucket_type_eor;
+
+/**
+ * Determine if a bucket is an End Of REQUEST (EOR) bucket
+ * @param e The bucket to inspect
+ * @return true or false
+ */
+#define AP_BUCKET_IS_EOR(e)         (e->type == &ap_bucket_type_eor)
+
+/**
+ * Make the bucket passed in an End Of REQUEST (EOR) bucket
+ * @param b The bucket to make into an EOR bucket
+ * @param r The request to destroy when this bucket is destroyed
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_eor_make(apr_bucket *b, request_rec *r);
+
+/**
+ * Create a bucket referring to an End Of REQUEST (EOR). This bucket
+ * holds a pointer to the request_rec, so that the request can be
+ * destroyed right after all of the output has been sent to the client.
+ *
+ * @param list The freelist from which this bucket should be allocated
+ * @param r The request to destroy when this bucket is destroyed
+ * @return The new bucket, or NULL if allocation failed
+ */
+AP_DECLARE(apr_bucket *) ap_bucket_eor_create(apr_bucket_alloc_t *list,
+                                              request_rec *r);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/modules/http/http_core.c b/modules/http/http_core.c
index c74b3ef791..1a76bc941a 100644
--- a/modules/http/http_core.c
+++ b/modules/http/http_core.c
@@ -142,7 +142,6 @@ static int ap_process_http_async_connection(conn_rec *c)
                 cs->state = CONN_STATE_READ_REQUEST_LINE;
             }
 
-            apr_pool_destroy(r->pool);
         }
         else {   /* ap_read_request failed - client may have closed */
             cs->state = CONN_STATE_LINGER;
@@ -182,7 +181,6 @@ static int ap_process_http_connection(conn_rec *c)
             break;
  
         ap_update_child_status(c->sbh, SERVER_BUSY_KEEPALIVE, r);
-        apr_pool_destroy(r->pool);
  
         if (ap_graceful_stop_signalled())
             break;
diff --git a/modules/http/http_request.c b/modules/http/http_request.c
index 1bae06f48c..07f37a070a 100644
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -191,25 +191,24 @@ AP_DECLARE(void) ap_die(int type, request_rec *r)
     ap_send_error_response(r_1st_err, recursive_error);
 }
 
-static void check_pipeline_flush(request_rec *r)
+static void check_pipeline_flush(conn_rec *c)
 {
     apr_bucket *e;
     apr_bucket_brigade *bb;
-    conn_rec *c = r->connection;
+
     /* ### if would be nice if we could PEEK without a brigade. that would
        ### allow us to defer creation of the brigade to when we actually
        ### need to send a FLUSH. */
-    bb = apr_brigade_create(r->pool, c->bucket_alloc);
+    bb = apr_brigade_create(c->pool, c->bucket_alloc);
 
     /* Flush the filter contents if:
      *
      *   1) the connection will be closed
      *   2) there isn't a request ready to be read
      */
-    /* ### shouldn't this read from the connection input filters? */
     /* ### is zero correct? that means "read one line" */
-    if (r->connection->keepalive != AP_CONN_CLOSE) {
-        if (ap_get_brigade(r->input_filters, bb, AP_MODE_EATCRLF, 
+    if (c->keepalive != AP_CONN_CLOSE) {
+        if (ap_get_brigade(c->input_filters, bb, AP_MODE_EATCRLF, 
                        APR_NONBLOCK_READ, 0) != APR_SUCCESS) {
             c->data_in_input_filters = 0;  /* we got APR_EOF or an error */
         }
@@ -228,12 +227,16 @@ static void check_pipeline_flush(request_rec *r)
          * if something hasn't been sent to the network yet.
          */
         APR_BRIGADE_INSERT_HEAD(bb, e);
-        ap_pass_brigade(r->connection->output_filters, bb);
+        ap_pass_brigade(c->output_filters, bb);
 }
 
+
 void ap_process_request(request_rec *r)
 {
     int access_status;
+    apr_bucket_brigade *bb;
+    apr_bucket *b;
+    conn_rec *c = r->connection;
 
     /* Give quick handlers a shot at serving the request on the fast
      * path, bypassing all of the other Apache hooks.
@@ -270,19 +273,25 @@ void ap_process_request(request_rec *r)
     else {
         ap_die(access_status, r);
     }
-    
-    /*
-     * We want to flush the last packet if this isn't a pipelining connection
-     * *before* we start into logging.  Suppose that the logging causes a DNS
-     * lookup to occur, which may have a high latency.  If we hold off on
-     * this packet, then it'll appear like the link is stalled when really
-     * it's the application that's stalled.
+
+    /* Send an EOR bucket through the output filter chain.  When
+     * this bucket is destroyed, the request will be logged and
+     * its pool will be freed
      */
-    check_pipeline_flush(r);
-    ap_update_child_status(r->connection->sbh, SERVER_BUSY_LOG, r);
-    ap_run_log_transaction(r);
+    bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);
+    b = ap_bucket_eor_create(r->connection->bucket_alloc, r);
+    APR_BRIGADE_INSERT_HEAD(bb, b);
+    ap_pass_brigade(r->connection->output_filters, bb);
+
+    /* From here onward, it is no longer safe to reference r
+     * or r->pool, because r->pool may have been destroyed
+     * already by the EOR bucket's cleanup function.
+     */
+
+    c->cs->state = CONN_STATE_WRITE_COMPLETION;
+    check_pipeline_flush(c);
     if (ap_extended_status)
-        ap_time_process_request(r->connection->sbh, STOP_PREQUEST);
+        ap_time_process_request(c->sbh, STOP_PREQUEST);
 }
 
 static apr_table_t *rename_original_env(apr_pool_t *p, apr_table_t *t)
diff --git a/server/Makefile.in b/server/Makefile.in
index 8efcb419d4..a4da96f548 100644
--- a/server/Makefile.in
+++ b/server/Makefile.in
@@ -14,7 +14,7 @@ LTLIBRARY_SOURCES = \
 	mpm_common.c util_charset.c util_debug.c util_xml.c \
 	util_filter.c util_pcre.c exports.c \
 	scoreboard.c error_bucket.c protocol.c core.c request.c provider.c \
-	eoc_bucket.c core_filters.c
+	eoc_bucket.c eor_bucket.c core_filters.c
 
 TARGETS = delete-exports $(LTLIBRARY_NAME) $(CORE_IMPLIB_FILE) export_vars.h httpd.exp
 
diff --git a/server/eor_bucket.c b/server/eor_bucket.c
new file mode 100644
index 0000000000..84e5bd21d5
--- /dev/null
+++ b/server/eor_bucket.c
@@ -0,0 +1,66 @@
+/* Copyright 2000-2005 The Apache Software Foundation or its licensors, as
+ * applicable.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "httpd.h"
+#include "http_request.h"
+
+static apr_status_t eor_bucket_read(apr_bucket *b, const char **str, 
+                                    apr_size_t *len, apr_read_type_e block)
+{
+    *str = NULL;
+    *len = 0;
+    return APR_SUCCESS;
+}
+
+AP_DECLARE(apr_bucket *) ap_bucket_eor_make(apr_bucket *b, request_rec *r)
+{
+    b->length      = 0;
+    b->start       = 0;
+    b->data        = r;
+    b->type        = &ap_bucket_type_eor;
+    
+    return b;
+}
+
+AP_DECLARE(apr_bucket *) ap_bucket_eor_create(apr_bucket_alloc_t *list,
+                                              request_rec *r)
+{
+    apr_bucket *b = apr_bucket_alloc(sizeof(*b), list);
+
+    APR_BUCKET_INIT(b);
+    b->free = apr_bucket_free;
+    b->list = list;
+    return ap_bucket_eor_make(b, r);
+}
+
+static void eor_bucket_destroy(void *data)
+{
+    request_rec *r = (request_rec *)data;
+    if (r != NULL) {
+        ap_run_log_transaction(r);
+        apr_pool_destroy(r->pool);
+    }
+}
+
+AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_type_eor = {
+    "EOR", 5, APR_BUCKET_METADATA,
+    eor_bucket_destroy,
+    eor_bucket_read,
+    apr_bucket_setaside_noop,
+    apr_bucket_split_notimpl,
+    apr_bucket_simple_copy
+};
+