Commit 94eee93b authored by Stefan Eissing's avatar Stefan Eissing
Browse files

On the 2.4.x branch:

Merge r1775813 from trunk:

Fix mod_h2/github issue #126: correct lifetime of data sent on temp pools

* modules/http2/h2_bucket_beam.c 
 - ignore send pools that are sub-pools of the existing one
 - added h2_beam_send_from() to allow explicit registering of the
   correct pool for the sending

* modules/http2/h2_bucket_beam.h
 - add prototype for h2_beam_send_from()

* modules/http2/h2_mplx.c
 - adding logging of output beam state

* modules/http2/h2_stream.c
 - register stream pool for sending data on input beam

* modules/http2/h2_task.c
 - register task pool on output beam on creation
 - adding trace logging

* modules/http2/h2_proxy_session.c
 - fixing a type in a comment while we're at it



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1775816 13f79535-47bb-0310-9956-ffa450edef68
parent 8e869d06
Loading
Loading
Loading
Loading
+4 −0
Original line number Diff line number Diff line
@@ -2,6 +2,10 @@

Changes with Apache 2.4.26

  *) mod_http2: fixes https://github.com/icing/mod_h2/issues/126 e.g. beam
     bucket lifetime handling when data is sent over temporary pools.
     [Stefan Eissing] 
  
Changes with Apache 2.4.25

  *) Fix some build issues related to various modules.
+24 −1
Original line number Diff line number Diff line
@@ -461,6 +461,12 @@ static void beam_set_send_pool(h2_bucket_beam *beam, apr_pool_t *pool)
{
    /* if the beam owner is the receiver, monitor sender pool lifetime */
    if (beam->owner == H2_BEAM_OWNER_RECV && beam->send_pool != pool) {
        if (beam->send_pool && pool 
            && apr_pool_is_ancestor(beam->send_pool, pool)) {
            /* when sender uses sub-pools to transmit data, stick
             * to the lifetime of the pool we already have. */
             return;
        }
        if (beam->send_pool) {
            apr_pool_cleanup_kill(beam->send_pool, beam, beam_send_cleanup);
        }
@@ -620,6 +626,8 @@ void h2_beam_abort(h2_bucket_beam *beam)
            r_purge_sent(beam);
            h2_blist_cleanup(&beam->send_list);
            report_consumption(beam, 0);
            ap_log_perror(APLOG_MARK, APLOG_WARNING, 0, beam->send_pool, 
                          "h2_beam(%d-%s): aborted", beam->id, beam->tag);
        }
        if (beam->m_cond) {
            apr_thread_cond_broadcast(beam->m_cond);
@@ -799,6 +807,17 @@ static apr_status_t append_bucket(h2_bucket_beam *beam,
    return APR_SUCCESS;
}

void h2_beam_send_from(h2_bucket_beam *beam, apr_pool_t *p)
{
    h2_beam_lock bl;
    /* Called from the red thread to add buckets to the beam */
    if (enter_yellow(beam, &bl) == APR_SUCCESS) {
        r_purge_sent(beam);
        beam_set_send_pool(beam, p);
        leave_yellow(beam, &bl);
    }
}

apr_status_t h2_beam_send(h2_bucket_beam *beam, 
                          apr_bucket_brigade *red_brigade, 
                          apr_read_type_e block)
@@ -809,8 +828,10 @@ apr_status_t h2_beam_send(h2_bucket_beam *beam,

    /* Called from the red thread to add buckets to the beam */
    if (enter_yellow(beam, &bl) == APR_SUCCESS) {
        ap_log_perror(APLOG_MARK, APLOG_WARNING, 0, beam->send_pool, 
                      "h2_beam(%d-%s): send", beam->id, beam->tag);
        r_purge_sent(beam);
        if (red_brigade) {
        if (red_brigade && !beam->send_pool) {
            beam_set_send_pool(beam, red_brigade->p);
        }
        
@@ -850,6 +871,8 @@ apr_status_t h2_beam_receive(h2_bucket_beam *beam,
    /* Called from the green thread to take buckets from the beam */
    if (enter_yellow(beam, &bl) == APR_SUCCESS) {
transfer:
        ap_log_perror(APLOG_MARK, APLOG_WARNING, 0, beam->send_pool, 
                      "h2_beam(%d-%s): receive", beam->id, beam->tag);
        if (beam->aborted) {
            if (beam->recv_buffer && !APR_BRIGADE_EMPTY(beam->recv_buffer)) {
                apr_brigade_cleanup(beam->recv_buffer);
+7 −0
Original line number Diff line number Diff line
@@ -254,6 +254,13 @@ apr_status_t h2_beam_send(h2_bucket_beam *beam,
                          apr_bucket_brigade *bb, 
                          apr_read_type_e block);

/**
 * Register the pool from which future buckets are send. This defines
 * the lifetime of the buckets, e.g. the pool should not be cleared/destroyed
 * until the data is no longer needed (or has been received).
 */
void h2_beam_send_from(h2_bucket_beam *beam, apr_pool_t *p);

/**
 * Receive buckets from the beam into the given brigade. Will return APR_EOF
 * when reading past an EOS bucket. Reads can be blocking until data is 
+10 −5
Original line number Diff line number Diff line
@@ -53,8 +53,8 @@ static void h2_beam_log(h2_bucket_beam *beam, int id, const char *msg,
        apr_size_t off = 0;
        
        off += apr_snprintf(buffer+off, H2_ALEN(buffer)-off, "cl=%d, ", beam->closed);
        off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "red", ", ", &beam->send_list);
        off += h2_util_bb_print(buffer+off, H2_ALEN(buffer)-off, "green", ", ", beam->recv_buffer);
        off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "to_send", ", ", &beam->send_list);
        off += h2_util_bb_print(buffer+off, H2_ALEN(buffer)-off, "recv_buffer", ", ", beam->recv_buffer);
        off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "hold", ", ", &beam->hold_list);
        off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "purge", "", &beam->purge_list);

@@ -703,8 +703,13 @@ static apr_status_t out_open(h2_mplx *m, int stream_id, h2_bucket_beam *beam)
        return APR_ECONNABORTED;
    }
    
    if (APLOGctrace2(m->c)) {
        h2_beam_log(beam, stream_id, "out_open", m->c, APLOG_TRACE2);
    }
    else {
        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, m->c,
                      "h2_mplx(%s): out open", task->id);
    }
    
    h2_beam_on_consumed(stream->output, stream_output_consumed, task);
    h2_beam_on_produced(stream->output, output_produced, m);
+1 −1
Original line number Diff line number Diff line
@@ -344,7 +344,7 @@ static void h2_proxy_stream_end_headers_out(h2_proxy_stream *stream)
        
        /* If USE_CANONICAL_NAME_OFF was configured for the proxy virtual host,
         * then the server name returned by ap_get_server_name() is the
         * origin server name (which does make too much sense with Via: headers)
         * origin server name (which doesn't make sense with Via: headers)
         * so we use the proxy vhost's name instead.
         */
        if (server_name == stream->r->hostname) {
Loading