Commit 6e5b7f3d authored by Yann Ylavic's avatar Yann Ylavic
Browse files

Merge r1818804, r1818951, r1818958, r1818960, r1819027, r1819214, r1820035 from trunk: 

mpm_event: close connections not reported as handled by any module.

This avoids losing track of them and leaking scoreboard entries.
PR 61551.


mpm_event: follow up to r1818804.

Address corner case where connection is aborted due to ap_run_pre_connection()
failure, and update comment about ap_run_process_connection() expected return
status and state.


mpm_event: follow up to r1818804 and r1818951.

Align comment and fix typos.


mpm_event: follow up to r1818804.

Allow DONE as a successful ap_run_process_connection() return value, for
instance h2_conn_run() and h2_task_process_conn() uses it, third-party
modules may too...


mpm_event: follow up to r1818804 and r1818951.

Be more correct in comment about CONN_STATE_WRITE_COMPLETION.
We currently have/need no state to simply wait for readability on a socket,
so the previous comment was misleading. Write completion can't be used for
a simple "wait for read event and come back to process_connection hooks".


mpm_event: follow up to r1818804 and r1818960.

Align mod_http2 with expected returned state from process_connection hooks in
async MPMs.
When the master connection is handled, enter CONN_STATE_LINGER in any case.


Add missing APLOGNO


Submitted by: ylavic, jailletc36
Reviewed by: ylavic, icing, covener


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

CHANGES

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

Changes with Apache 2.4.30

 

  *) mpm_event: close connections not reported as handled by any module to

     avoid losing track of them and leaking scoreboard entries.  PR 61551.

     [Yann Ylavic]



  *) core: A signal received while stopping could have crashed the main

     process.  PR 61558.  [Yann Ylavic]

modules/http2/h2_conn.c

+4 −17
Original line number Diff line number Diff line
@@ -255,23 +255,10 @@ apr_status_t h2_conn_run(struct h2_ctx *ctx, conn_rec *c) 
             && mpm_state != AP_MPMQ_STOPPING);



    if (c->cs) {

        switch (session->state) {

            case H2_SESSION_ST_INIT:

            case H2_SESSION_ST_CLEANUP:

            case H2_SESSION_ST_DONE:

            case H2_SESSION_ST_IDLE:

                c->cs->state = CONN_STATE_WRITE_COMPLETION;

                break;

            case H2_SESSION_ST_BUSY:

            case H2_SESSION_ST_WAIT:

            default:

                c->cs->state = CONN_STATE_HANDLER;

                break;

                

        }

        c->cs->state = CONN_STATE_LINGER;

    }



    return DONE;

    return APR_SUCCESS;

}



apr_status_t h2_conn_pre_close(struct h2_ctx *ctx, conn_rec *c)

modules/http2/h2_h2.c

+3 −2
Original line number Diff line number Diff line
@@ -669,10 +669,11 @@ int h2_h2_process_conn(conn_rec* c) 
            ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, c, "conn_setup");

            if (status != APR_SUCCESS) {

                h2_ctx_clear(c);

                return status;

                return !OK;

            }

        }

        return h2_conn_run(ctx, c);

        h2_conn_run(ctx, c);

        return OK;

    }

    

    ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, "h2_h2, declined");

modules/http2/h2_switch.c

+2 −3
Original line number Diff line number Diff line
@@ -185,13 +185,12 @@ static int h2_protocol_switch(conn_rec *c, request_rec *r, server_rec *s, 
                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, status, r, APLOGNO(03088)

                              "session setup");

                h2_ctx_clear(c);

                return status;

                return !OK;

            }

            

            h2_conn_run(ctx, c);

            return DONE;

        }

        return DONE;

        return OK;

    }

    

    return DECLINED;

server/mpm/event/event.c

+60 −18
Original line number Diff line number Diff line
@@ -993,14 +993,12 @@ static void process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * soc 
        c->current_thread = thd;

        /* Subsequent request on a conn, and thread number is part of ID */

        c->id = conn_id;



        if (c->aborted) {

            cs->pub.state = CONN_STATE_LINGER;

        }

    }



    if (cs->pub.state == CONN_STATE_LINGER) {

    rc = OK;

    if (c->aborted || cs->pub.state == CONN_STATE_LINGER) {

        /* do lingering close below */

        cs->pub.state = CONN_STATE_LINGER;

    }

    else if (c->clogging_input_filters) {

        /* Since we have an input filter which 'clogs' the input stream,
@@ -1010,20 +1008,54 @@ static void process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * soc 
         * otherwise write, should set the sense appropriately.

         */

        apr_atomic_inc32(&clogged_count);

        ap_run_process_connection(c);

        if (cs->pub.state != CONN_STATE_SUSPENDED) {

            cs->pub.state = CONN_STATE_LINGER;

        }

        rc = ap_run_process_connection(c);

        apr_atomic_dec32(&clogged_count);

        if (rc == DONE) {

            rc = OK;

        }

    }

    else if (cs->pub.state == CONN_STATE_READ_REQUEST_LINE) {

read_request:

        ap_run_process_connection(c);



        /* state will be updated upon return

         * fall thru to either wait for readability/timeout or

         * do lingering close

         */

        rc = ap_run_process_connection(c);

        if (rc == DONE) {

            rc = OK;

        }

    }

    /*

     * The process_connection hooks above should set the connection state

     * appropriately upon return, for event MPM to either:

     * - do lingering close (CONN_STATE_LINGER),

     * - wait for readability of the next request with respect to the keepalive

     *   timeout (CONN_STATE_CHECK_REQUEST_LINE_READABLE),

     * - keep flushing the output filters stack in nonblocking mode, and then

     *   if required wait for read/write-ability of the underlying socket with

     *   respect to its own timeout (CONN_STATE_WRITE_COMPLETION); since write

     *   completion at some point may require reads (e.g. SSL_ERROR_WANT_READ),

     *   an output filter can set the sense to CONN_SENSE_WANT_READ at any time

     *   for event MPM to do the right thing,

     * - suspend the connection (SUSPENDED) such that it now interracts with

     *   the MPM through suspend/resume_connection() hooks, and/or registered

     *   poll callbacks (PT_USER), and/or registered timed callbacks triggered

     *   by timer events.

     * If a process_connection hook returns an error or no hook sets the state

     * to one of the above expected value, we forcibly close the connection w/

     * CONN_STATE_LINGER.  This covers the cases where no process_connection

     * hook executes (DECLINED), or one returns OK w/o touching the state (i.e.

     * CONN_STATE_READ_REQUEST_LINE remains after the call) which can happen

     * with third-party modules not updated to work specifically with event MPM

     * while this was expected to do lingering close unconditionally with

     * worker or prefork MPMs for instance.

     */

    if (rc != OK || (cs->pub.state != CONN_STATE_LINGER

                     && cs->pub.state != CONN_STATE_WRITE_COMPLETION

                     && cs->pub.state != CONN_STATE_CHECK_REQUEST_LINE_READABLE

                     && cs->pub.state != CONN_STATE_SUSPENDED)) {

        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(10111)

                      "process_socket: connection processing %s: closing",

                      rc ? apr_psprintf(c->pool, "returned error %i", rc)

                         : apr_psprintf(c->pool, "unexpected state %i",

                                                 (int)cs->pub.state));

        cs->pub.state = CONN_STATE_LINGER;

    }



    if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) {
@@ -1046,10 +1078,20 @@ read_request: 
             */

            cs->queue_timestamp = apr_time_now();

            notify_suspend(cs);

            cs->pfd.reqevents = (

                    cs->pub.sense == CONN_SENSE_WANT_READ ? APR_POLLIN :

                            APR_POLLOUT) | APR_POLLHUP | APR_POLLERR;



            if (cs->pub.sense == CONN_SENSE_WANT_READ) {

                cs->pfd.reqevents = APR_POLLIN;

            }

            else {

                cs->pfd.reqevents = APR_POLLOUT;

            }

            /* POLLHUP/ERR are usually returned event only (ignored here), but

             * some pollset backends may require them in reqevents to do the

             * right thing, so it shouldn't hurt.

             */

            cs->pfd.reqevents |= APR_POLLHUP | APR_POLLERR;

            cs->pub.sense = CONN_SENSE_DEFAULT;



            apr_thread_mutex_lock(timeout_mutex);

            TO_QUEUE_APPEND(cs->sc->wc_q, cs);

            rc = apr_pollset_add(event_pollset, &cs->pfd);