Mailing-List: contact cvs-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <20051024023954.82900.qmail@minotaur.apache.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r327925 - in /httpd/httpd/trunk: CHANGES
 include/http_request.h
 modules/http/http_core.c modules/http/http_request.c server/Makefile.in
 server/eor_bucket.c
Date: Mon, 24 Oct 2005 02:39:52 -0000
To: cvs@httpd.apache.org
From: brianp@apache.org

Author: brianp
Date: Sun Oct 23 19:39:49 2005
New Revision: 327925

URL: http://svn.apache.org/viewcvs?rev=327925&view=rev
Log:
Redesign of request cleanup and logging to use End-Of-Request bucket
(backport from async-dev branch to 2.3 trunk)

Added:
    httpd/httpd/trunk/server/eor_bucket.c
Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/include/http_request.h
    httpd/httpd/trunk/modules/http/http_core.c
    httpd/httpd/trunk/modules/http/http_request.c
    httpd/httpd/trunk/server/Makefile.in

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/CHANGES?rev=327925&r1=327924&r2=327925&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sun Oct 23 19:39:49 2005
@@ -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]
 

Modified: httpd/httpd/trunk/include/http_request.h
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/include/http_request.h?rev=327925&r1=327924&r2=327925&view=diff
==============================================================================
--- httpd/httpd/trunk/include/http_request.h (original)
+++ httpd/httpd/trunk/include/http_request.h Sun Oct 23 19:39:49 2005
@@ -237,12 +237,21 @@
 
 #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
@@ -353,6 +362,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
 }

Modified: httpd/httpd/trunk/modules/http/http_core.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/http/http_core.c?rev=327925&r1=327924&r2=327925&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_core.c (original)
+++ httpd/httpd/trunk/modules/http/http_core.c Sun Oct 23 19:39:49 2005
@@ -142,7 +142,6 @@
                 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 @@
             break;
  
         ap_update_child_status(c->sbh, SERVER_BUSY_KEEPALIVE, r);
-        apr_pool_destroy(r->pool);
  
         if (ap_graceful_stop_signalled())
             break;

Modified: httpd/httpd/trunk/modules/http/http_request.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/http/http_request.c?rev=327925&r1=327924&r2=327925&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/http/http_request.c (original)
+++ httpd/httpd/trunk/modules/http/http_request.c Sun Oct 23 19:39:49 2005
@@ -191,25 +191,24 @@
     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 @@
          * 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 @@
     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)

Modified: httpd/httpd/trunk/server/Makefile.in
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/server/Makefile.in?rev=327925&r1=327924&r2=327925&view=diff
==============================================================================
--- httpd/httpd/trunk/server/Makefile.in (original)
+++ httpd/httpd/trunk/server/Makefile.in Sun Oct 23 19:39:49 2005
@@ -14,7 +14,7 @@
 	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
 

Added: httpd/httpd/trunk/server/eor_bucket.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/server/eor_bucket.c?rev=327925&view=auto
==============================================================================
--- httpd/httpd/trunk/server/eor_bucket.c (added)
+++ httpd/httpd/trunk/server/eor_bucket.c Sun Oct 23 19:39:49 2005
@@ -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
+};
+