* Move the tasks of eor_bucket_destroy (status update, transaction logging) to

  a cleanup function that is registered with the request pool.
  eor_bucket_destroy only destroys the request pool if there is still a request
  at this point of time which triggers the call of the cleanup function.

  This is needed as the eor bucket could be (and usually is) allocated from
  a pool different from the request pool (the connection pool in this case).
  This can lead to situations where eor_bucket_destroy is called at a point of
  time when the request pool had been already destroyed. But the status update
  and the transaction logging rely on the request pool.

  In practise this happens with the worker MPM if the client refuses to read
  the response from the handler. Then the unsent data (including the
  eor bucket) remains in a brigade allocated from the connection pool which
  is the parent of the request pool. This brigade is cleaned up during the
  cleanup of the ptrans pool in worker_thread. As the request pool gets
  destroyed during this cleanup operation *before* the brigade gets cleaned up
  eor_bucket_destroy starts the status update and the transaction logging with
  a destroyed request pool which leads to a segfault.

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

diff --git a/server/eor_bucket.c b/server/eor_bucket.c
index 3dbe7012b5b9fc0659b0f3dbba596db6f56a7593..b758bc92436f99b8714e488bb044875519293577 100644 (file)
--- a/server/eor_bucket.c
+++ b/server/eor_bucket.c
@@ -19,6 +19,27 @@
 #include "http_protocol.h"
 #include "scoreboard.h"
 
+static apr_status_t eor_bucket_cleanup(void *data)
+{
+    apr_bucket *b = (apr_bucket *)data;
+    request_rec *r = (request_rec *)b->data;
+
+    if (r != NULL) {
+        /*
+         * If eor_bucket_destroy is called after us, this prevents
+         * eor_bucket_destroy from trying to destroy the pool again.
+         */
+        b->data = NULL;
+        /* Update child status and log the transaction */
+        ap_update_child_status(r->connection->sbh, SERVER_BUSY_LOG, r);
+        ap_run_log_transaction(r);
+        if (ap_extended_status) {
+            ap_increment_counts(r->connection->sbh, r);
+        }
+    }
+    return APR_SUCCESS;
+}
+
 static apr_status_t eor_bucket_read(apr_bucket *b, const char **str,
                                     apr_size_t *len, apr_read_type_e block)
 {
@@ -45,18 +66,26 @@ AP_DECLARE(apr_bucket *) ap_bucket_eor_create(apr_bucket_alloc_t *list,
     APR_BUCKET_INIT(b);
     b->free = apr_bucket_free;
     b->list = list;
+    if (r) {
+        /*
+         * Register a cleanup for the request pool as the eor bucket could
+         * have been allocated from a different pool then the request pool
+         * e.g. the parent pool of the request pool. In this case
+         * eor_bucket_destroy might be called at a point of time when the
+         * request pool had been already destroyed.
+         */
+        apr_pool_cleanup_register(r->pool, (void *)b, eor_bucket_cleanup,
+                                  apr_pool_cleanup_null);
+    }
     return ap_bucket_eor_make(b, r);
 }
 
 static void eor_bucket_destroy(void *data)
 {
     request_rec *r = (request_rec *)data;
-    if (r != NULL) {
-        ap_update_child_status(r->connection->sbh, SERVER_BUSY_LOG, r);
-        ap_run_log_transaction(r);
-        if (ap_extended_status) {
-            ap_increment_counts(r->connection->sbh, r);
-        }
+
+    if (r) {
+        /* eor_bucket_cleanup will be called when the pool gets destroyed */
         apr_pool_destroy(r->pool);
     }
 }