Commit 43b7cc98 authored by Stefan Eissing's avatar Stefan Eissing
Browse files

On the 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/trunk@1775813 13f79535-47bb-0310-9956-ffa450edef68
parent 64086b6e

CHANGES

+4 −0
Original line number Diff line number Diff line 
                                                         -*- coding: utf-8 -*-

Changes with Apache 2.5.0



  *) 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] 

  

  *) mod_proxy_{ajp,fcgi}: Fix a possible crash when reusing an established

     backend connection, happening with LogLevel trace2 or higher configured,

     or at any log level with compilers not detected as C99 compliant (e.g.

modules/http2/h2_bucket_beam.c

+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);

modules/http2/h2_bucket_beam.h

+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

modules/http2/h2_mplx.c

+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);

modules/http2/h2_proxy_session.c

+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) {