From c29746847495bf07f52394381f47c95b70f83247 Mon Sep 17 00:00:00 2001 From: Jim Jagielski Date: Mon, 23 Jul 2012 12:35:23 +0000 Subject: [PATCH] Merge r1294356 from trunk: Take care not to call ap_start_lingering_close from the listener thread, because it may block when flushing data to the client. From the listener thread, do a lingering close without flushing. This is OK because we only do this if there has been an error during write completion or if our send buffers are empty because we are in keep-alive. PR: 52229 Submitted by: sf Reviewed/backported by: jim git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1364613 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 3 + STATUS | 6 -- include/httpd.h | 8 ++- server/mpm/event/event.c | 139 +++++++++++++++++++++++---------------- 4 files changed, 91 insertions(+), 65 deletions(-) diff --git a/CHANGES b/CHANGES index 4b1efaf6a6..3cb2c9eebc 100644 --- a/CHANGES +++ b/CHANGES @@ -8,6 +8,9 @@ Changes with Apache 2.4.3 possible XSS for a site where untrusted users can upload files to a location with MultiViews enabled. [Niels Heinen ] + *) mpm_event: Don't do a blocking write when starting a lingering close + from the listener thread. PR 52229. [Stefan Fritsch] + *) mod_so: If a filename without slashes is specified for LoadFile or LoadModule and the file cannot be found in the server root directory, try to use the standard dlopen() search path. [Stefan Fritsch] diff --git a/STATUS b/STATUS index 7d14767838..2ad56780f1 100644 --- a/STATUS +++ b/STATUS @@ -88,12 +88,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * mpm_event: Don't call ap_start_lingering_close from the listener thread - because it may block - PR: 52229 - trunk patch: http://svn.apache.org/viewvc?view=revision&revision=1294356 - 2.4.x patch: trunk patch works (needs CHANGES entry) - +1: sf, rjung, jim PATCHES PROPOSED TO BACKPORT FROM TRUNK: diff --git a/include/httpd.h b/include/httpd.h index c5f3872ac7..e718bfc7c1 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -1161,6 +1161,8 @@ struct conn_rec { /** * Enumeration of connection states + * The two states CONN_STATE_LINGER_NORMAL and CONN_STATE_LINGER_SHORT may + * only be set by the MPM. Use CONN_STATE_LINGER outside of the MPM. */ typedef enum { CONN_STATE_CHECK_REQUEST_LINE_READABLE, @@ -1168,9 +1170,9 @@ typedef enum { CONN_STATE_HANDLER, CONN_STATE_WRITE_COMPLETION, CONN_STATE_SUSPENDED, - CONN_STATE_LINGER, - CONN_STATE_LINGER_NORMAL, - CONN_STATE_LINGER_SHORT + CONN_STATE_LINGER, /* connection may be closed with lingering */ + CONN_STATE_LINGER_NORMAL, /* MPM has started lingering close with normal timeout */ + CONN_STATE_LINGER_SHORT /* MPM has started lingering close with short timeout */ } conn_state_e; /** diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index 007d887f7d..0934e0dc0c 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -742,70 +742,96 @@ static void set_signals(void) #endif } +static int start_lingering_close_common(event_conn_state_t *cs) +{ + apr_status_t rv; + struct timeout_queue *q; + apr_socket_t *csd = cs->pfd.desc.s; +#ifdef AP_DEBUG + { + rv = apr_socket_timeout_set(csd, 0); + AP_DEBUG_ASSERT(rv == APR_SUCCESS); + } +#else + apr_socket_timeout_set(csd, 0); +#endif + /* + * If some module requested a shortened waiting period, only wait for + * 2s (SECONDS_TO_LINGER). This is useful for mitigating certain + * DoS attacks. + */ + if (apr_table_get(cs->c->notes, "short-lingering-close")) { + cs->expiration_time = + apr_time_now() + apr_time_from_sec(SECONDS_TO_LINGER); + q = &short_linger_q; + cs->pub.state = CONN_STATE_LINGER_SHORT; + } + else { + cs->expiration_time = + apr_time_now() + apr_time_from_sec(MAX_SECS_TO_LINGER); + q = &linger_q; + cs->pub.state = CONN_STATE_LINGER_NORMAL; + } + apr_thread_mutex_lock(timeout_mutex); + TO_QUEUE_APPEND(*q, cs); + cs->pfd.reqevents = APR_POLLIN | APR_POLLHUP | APR_POLLERR; + rv = apr_pollset_add(event_pollset, &cs->pfd); + apr_thread_mutex_unlock(timeout_mutex); + if (rv != APR_SUCCESS && !APR_STATUS_IS_EEXIST(rv)) { + ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf, + "start_lingering_close: apr_pollset_add failure"); + apr_thread_mutex_lock(timeout_mutex); + TO_QUEUE_REMOVE(*q, cs); + apr_thread_mutex_unlock(timeout_mutex); + apr_socket_close(cs->pfd.desc.s); + apr_pool_clear(cs->p); + ap_push_pool(worker_queue_info, cs->p); + return 0; + } + return 1; +} + /* - * close our side of the connection + * Close our side of the connection, flushing data to the client first. * Pre-condition: cs is not in any timeout queue and not in the pollset, * timeout_mutex is not locked * return: 0 if connection is fully closed, * 1 if connection is lingering - * may be called by listener or by worker thread + * May only be called by worker thread. */ -static int start_lingering_close(event_conn_state_t *cs) +static int start_lingering_close_blocking(event_conn_state_t *cs) { - apr_status_t rv; - if (ap_start_lingering_close(cs->c)) { apr_pool_clear(cs->p); ap_push_pool(worker_queue_info, cs->p); return 0; } - else { - apr_socket_t *csd = ap_get_conn_socket(cs->c); - struct timeout_queue *q; + return start_lingering_close_common(cs); +} -#ifdef AP_DEBUG - { - rv = apr_socket_timeout_set(csd, 0); - AP_DEBUG_ASSERT(rv == APR_SUCCESS); - } -#else - apr_socket_timeout_set(csd, 0); -#endif - /* - * If some module requested a shortened waiting period, only wait for - * 2s (SECONDS_TO_LINGER). This is useful for mitigating certain - * DoS attacks. - */ - if (apr_table_get(cs->c->notes, "short-lingering-close")) { - cs->expiration_time = - apr_time_now() + apr_time_from_sec(SECONDS_TO_LINGER); - q = &short_linger_q; - cs->pub.state = CONN_STATE_LINGER_SHORT; - } - else { - cs->expiration_time = - apr_time_now() + apr_time_from_sec(MAX_SECS_TO_LINGER); - q = &linger_q; - cs->pub.state = CONN_STATE_LINGER_NORMAL; - } - apr_thread_mutex_lock(timeout_mutex); - TO_QUEUE_APPEND(*q, cs); - cs->pfd.reqevents = APR_POLLIN | APR_POLLHUP | APR_POLLERR; - rv = apr_pollset_add(event_pollset, &cs->pfd); - apr_thread_mutex_unlock(timeout_mutex); - if (rv != APR_SUCCESS && !APR_STATUS_IS_EEXIST(rv)) { - ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf, - "start_lingering_close: apr_pollset_add failure"); - apr_thread_mutex_lock(timeout_mutex); - TO_QUEUE_REMOVE(*q, cs); - apr_thread_mutex_unlock(timeout_mutex); - apr_socket_close(cs->pfd.desc.s); - apr_pool_clear(cs->p); - ap_push_pool(worker_queue_info, cs->p); - return 0; - } +/* + * Close our side of the connection, NOT flushing data to the client. + * This should only be called if there has been an error or if we know + * that our send buffers are empty. + * Pre-condition: cs is not in any timeout queue and not in the pollset, + * timeout_mutex is not locked + * return: 0 if connection is fully closed, + * 1 if connection is lingering + * may be called by listener thread + */ +static int start_lingering_close_nonblocking(event_conn_state_t *cs) +{ + conn_rec *c = cs->c; + apr_socket_t *csd = cs->pfd.desc.s; + + if (c->aborted + || apr_socket_shutdown(csd, APR_SHUTDOWN_WRITE) != APR_SUCCESS) { + apr_socket_close(csd); + apr_pool_clear(cs->p); + ap_push_pool(worker_queue_info, cs->p); + return 0; } - return 1; + return start_lingering_close_common(cs); } /* @@ -969,7 +995,7 @@ read_request: } if (cs->pub.state == CONN_STATE_LINGER) { - if (!start_lingering_close(cs)) + if (!start_lingering_close_blocking(cs)) return; } else if (cs->pub.state == CONN_STATE_CHECK_REQUEST_LINE_READABLE) { @@ -1471,7 +1497,7 @@ static void * APR_THREAD_FUNC listener_thread(apr_thread_t * thd, void *dummy) ap_log_error(APLOG_MARK, APLOG_ERR, rc, ap_server_conf, "pollset remove failed"); apr_thread_mutex_unlock(timeout_mutex); - start_lingering_close(cs); + start_lingering_close_nonblocking(cs); break; } @@ -1482,7 +1508,7 @@ static void * APR_THREAD_FUNC listener_thread(apr_thread_t * thd, void *dummy) * re-connect to a different process. */ if (!have_idle_worker) { - start_lingering_close(cs); + start_lingering_close_nonblocking(cs); break; } rc = push2worker(out_pfd, event_pollset); @@ -1622,14 +1648,15 @@ static void * APR_THREAD_FUNC listener_thread(apr_thread_t * thd, void *dummy) keepalive_q.count); process_timeout_queue(&keepalive_q, timeout_time + ap_server_conf->keep_alive_timeout, - start_lingering_close); + start_lingering_close_nonblocking); } else { process_timeout_queue(&keepalive_q, timeout_time, - start_lingering_close); + start_lingering_close_nonblocking); } /* Step 2: write completion timeouts */ - process_timeout_queue(&write_completion_q, timeout_time, start_lingering_close); + process_timeout_queue(&write_completion_q, timeout_time, + start_lingering_close_nonblocking); /* Step 3: (normal) lingering close completion timeouts */ process_timeout_queue(&linger_q, timeout_time, stop_lingering_close); /* Step 4: (short) lingering close completion timeouts */ -- 2.40.0