From: Yann Ylavic Date: Thu, 21 Dec 2017 16:44:37 +0000 (+0000) Subject: mpm_event: follow up to r1818804. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0fda557c3902893b376eebbd35e07da9615a1fe0;p=apache 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. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1818951 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index cb730a3762..8811b08661 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -1032,14 +1032,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, @@ -1050,29 +1048,45 @@ static void process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * soc */ apr_atomic_inc32(&clogged_count); rc = ap_run_process_connection(c); - if (rc || cs->pub.state != CONN_STATE_SUSPENDED) { - cs->pub.state = CONN_STATE_LINGER; - } apr_atomic_dec32(&clogged_count); } else if (cs->pub.state == CONN_STATE_READ_REQUEST_LINE) { read_request: rc = ap_run_process_connection(c); - - /* State will be updated upon successful return: fall thru to either - * wait for readability/timeout, do write completion or lingering - * close. But forcibly close the connection if the run failed (handler - * raised an error for it) or the state is something unexpected at the - * MPM level (meaning that no module handled it and we can't do much - * here; note that if a handler wants mpm_event to keep POLLIN for the - * rest of the request line it should use CHECK_REQUEST_LINE_READABLE - * and not simply return OK with the initial READ_REQUEST_LINE state). - */ - if (rc || (cs->pub.state != CONN_STATE_CHECK_REQUEST_LINE_READABLE - && cs->pub.state != CONN_STATE_WRITE_COMPLETION - && cs->pub.state != CONN_STATE_SUSPENDED)) { - cs->pub.state = CONN_STATE_LINGER; - } + } + /* + * 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 according to the + * keepalive timeout (CONN_STATE_CHECK_REQUEST_LINE_READABLE), + * - wait for read/write-ability on the underlying socket according to + * its timeout (CONN_STATE_WRITE_COMPLETION, a legacy name which can + * be also used for readability by setting CONN_SENSE_WANT_READ), + * - suspend the connection 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 + * the state to one of the above expected value, we forcibly close the + * connection (=> 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 for third-party modules not updated to + * work specifically with event/async MPMs 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_CHECK_REQUEST_LINE_READABLE + && cs->pub.state != CONN_STATE_WRITE_COMPLETION + && cs->pub.state != CONN_STATE_SUSPENDED)) { + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO() + "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) {