Merge r1666998 from trunk:

core: Cleanup the request soon/even if some output filter fails to
handle the EOR bucket.

Submitted by: ylavic
Reviewed/backported by: jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1681113 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index f759fd87d451e7c2853c919b30d935c4ad1fbef3..dea2424d3ad1b2ab6202b840e0a725ab146d7f0a 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -35,6 +35,9 @@ Changes with Apache 2.4.13
      or framework is unable to return Location in the internal-redirect
      form.  [Jeff Trawick]
 
+  *) core: Cleanup the request soon/even if some output filter fails to
+     handle the EOR bucket.  [Yann Ylavic]
+
   *) mpm_event: Allow for timer events duplicates. [Jim Jagielski, Yann Ylavic]
 
   *) mod_proxy, mod_ssl, mod_cache_socache, mod_socache_*: Support machine

diff --git a/STATUS b/STATUS
index a789dea9d2e031878e8242d6c08cf76a7a52763b..a0c97a775d2495c6ea68af4ceef437c892bb3e2e 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -112,13 +112,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
      2.4.x patch: trunk works (modulo CHANGES)
      +1: ylavic, minfrin, jim
 
-  *) core: Cleanup the request soon/even if some output filter fails to
-     handle the EOR bucket.
-     trunk patch: http://svn.apache.org/r1666998
-     2.4.x patch: trunk works (modulo CHANGES)
-     +1: ylavic, minfrin, jim
-
-
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

diff --git a/modules/http/http_request.c b/modules/http/http_request.c
index cdfec8b5689a8b5b536c7de7d36a4e1f8549624c..9f23a9ddec05f9d3a6e6ca58b799540a1960eb6e 100644 (file)
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -216,12 +216,12 @@ AP_DECLARE(void) ap_die(int type, request_rec *r)
     ap_send_error_response(r_1st_err, recursive_error);
 }
 
-static void check_pipeline(conn_rec *c)
+static void check_pipeline(conn_rec *c, apr_bucket_brigade *bb)
 {
-    if (c->keepalive != AP_CONN_CLOSE) {
+    if (c->keepalive != AP_CONN_CLOSE && !c->aborted) {
         apr_status_t rv;
-        apr_bucket_brigade *bb = apr_brigade_create(c->pool, c->bucket_alloc);
 
+        AP_DEBUG_ASSERT(APR_BRIGADE_EMPTY(bb));
         rv = ap_get_brigade(c->input_filters, bb, AP_MODE_SPECULATIVE,
                             APR_NONBLOCK_READ, 1);
         if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(bb)) {
@@ -234,7 +234,6 @@ static void check_pipeline(conn_rec *c)
         else {
             c->data_in_input_filters = 1;
         }
-        apr_brigade_destroy(bb);
     }
 }
 
@@ -249,20 +248,30 @@ AP_DECLARE(void) ap_process_request_after_handler(request_rec *r)
      * this bucket is destroyed, the request will be logged and
      * its pool will be freed
      */
-    bb = apr_brigade_create(r->connection->pool, r->connection->bucket_alloc);
-    b = ap_bucket_eor_create(r->connection->bucket_alloc, r);
+    bb = apr_brigade_create(c->pool, c->bucket_alloc);
+    b = ap_bucket_eor_create(c->bucket_alloc, r);
     APR_BRIGADE_INSERT_HEAD(bb, b);
 
-    ap_pass_brigade(r->connection->output_filters, bb);
+    ap_pass_brigade(c->output_filters, bb);
+    
+    /* The EOR bucket has either been handled by an output filter (eg.
+     * deleted or moved to a buffered_bb => no more in bb), or an error
+     * occured before that (eg. c->aborted => still in bb) and we ought
+     * to destroy it now. So cleanup any remaining bucket along with
+     * the orphan request (if any).
+     */
+    apr_brigade_cleanup(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.
      */
 
+    check_pipeline(c, bb);
+    apr_brigade_destroy(bb);
     if (c->cs)
-        c->cs->state = CONN_STATE_WRITE_COMPLETION;
-    check_pipeline(c);
+        c->cs->state = (c->aborted) ? CONN_STATE_LINGER
+                                    : CONN_STATE_WRITE_COMPLETION;
     AP_PROCESS_REQUEST_RETURN((uintptr_t)r, r->uri, r->status);
     if (ap_extended_status) {
         ap_time_process_request(c->sbh, STOP_PREQUEST);

diff --git a/server/core_filters.c b/server/core_filters.c
index b535887236cf9976d92f71b4c5127220b5e90cd7..61dc2f0ac43d820ce55f6508efe0ac17da5ba24a 100644 (file)
--- a/server/core_filters.c
+++ b/server/core_filters.c
@@ -468,17 +468,16 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb)
     if (new_bb == NULL) {
         rv = send_brigade_nonblocking(net->client_socket, bb,
                                       &(ctx->bytes_written), c);
-        if (APR_STATUS_IS_EAGAIN(rv)) {
-            rv = APR_SUCCESS;
-        }
-        else if (rv != APR_SUCCESS) {
+        if (rv != APR_SUCCESS && !APR_STATUS_IS_EAGAIN(rv)) {
             /* The client has aborted the connection */
             ap_log_cerror(APLOG_MARK, APLOG_TRACE1, rv, c,
                           "core_output_filter: writing data to the network");
+            apr_brigade_cleanup(bb);
             c->aborted = 1;
+            return rv;
         }
         setaside_remaining_output(f, ctx, bb, c);
-        return rv;
+        return APR_SUCCESS;
     }
 
     bytes_in_brigade = 0;
@@ -547,6 +546,7 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb)
             /* The client has aborted the connection */
             ap_log_cerror(APLOG_MARK, APLOG_TRACE1, rv, c,
                           "core_output_filter: writing data to the network");
+            apr_brigade_cleanup(bb);
             c->aborted = 1;
             return rv;
         }
@@ -560,6 +560,7 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *new_bb)
             /* The client has aborted the connection */
             ap_log_cerror(APLOG_MARK, APLOG_TRACE1, rv, c,
                           "core_output_filter: writing data to the network");
+            apr_brigade_cleanup(bb);
             c->aborted = 1;
             return rv;
         }