From b1102a1ca550e7b35b1bd5f904f12da0b8a4ad9a Mon Sep 17 00:00:00 2001 From: Jim Jagielski Date: Thu, 21 Dec 2017 18:49:24 +0000 Subject: [PATCH] Merge r1809273, r1814719 from trunk: event: better apr_pollset_add() failure handling to avoid an (very unlikely) worker vs listener race condition. Follow up to r1809273: CHANGES entry. Submitted by: ylavic Reviewed by: ylavic, jim, covener git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1818963 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 4 ++++ STATUS | 8 -------- server/mpm/event/event.c | 26 ++++++++++++-------------- 3 files changed, 16 insertions(+), 22 deletions(-) diff --git a/CHANGES b/CHANGES index c71b04bf56..3a0f27bafd 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,10 @@ -*- coding: utf-8 -*- Changes with Apache 2.4.30 + *) mpm_event: avoid a very unlikely race condition between the listener and + the workers when the latter fails to add a connection to the pollset. + [Yann Ylavic] + *) mod_macro: fix usability of globally defined macros in .htaccess files. PR 57525. [Jose Kahan , Yann Ylavic] diff --git a/STATUS b/STATUS index 87bafa7c80..9ff8e1d869 100644 --- a/STATUS +++ b/STATUS @@ -118,14 +118,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - *) mpm_event: avoid a very unlikely race condition between the listener and - the workers when the latter fails to add a connection to the pollset. - trunk patch: http://svn.apache.org/r1809273 - http://svn.apache.org/r1814719 (CHANGES only) - 2.4.x patch: trunk works (modulo CHANGES) - svn merge -c 1809273,1814719 ^/httpd/httpd/trunk . - +1: ylavic, jim, covener - *) core: silently ignore a not existent file path when IncludeOptional is used. trunk patch: http://svn.apache.org/r1814968 2.4.x patch: trunk works (modulo CHANGES) diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c index 891ed3b313..bd3ad3c998 100644 --- a/server/mpm/event/event.c +++ b/server/mpm/event/event.c @@ -306,6 +306,7 @@ static void TO_QUEUE_APPEND(struct timeout_queue *q, event_conn_state_t *el) static void TO_QUEUE_REMOVE(struct timeout_queue *q, event_conn_state_t *el) { APR_RING_REMOVE(el, timeout_list); + APR_RING_ELEM_INIT(el, timeout_list); apr_atomic_dec32(q->total); --q->count; } @@ -797,17 +798,16 @@ static int start_lingering_close_blocking(event_conn_state_t *cs) apr_thread_mutex_lock(timeout_mutex); TO_QUEUE_APPEND(q, cs); 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, APLOGNO(03092) - "start_lingering_close: apr_pollset_add failure"); - apr_thread_mutex_lock(timeout_mutex); TO_QUEUE_REMOVE(q, cs); apr_thread_mutex_unlock(timeout_mutex); + ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf, APLOGNO(03092) + "start_lingering_close: apr_pollset_add failure"); apr_socket_close(cs->pfd.desc.s); ap_push_pool(worker_queue_info, cs->p); return 0; } + apr_thread_mutex_unlock(timeout_mutex); return 1; } @@ -1053,17 +1053,18 @@ read_request: apr_thread_mutex_lock(timeout_mutex); TO_QUEUE_APPEND(cs->sc->wc_q, cs); rc = apr_pollset_add(event_pollset, &cs->pfd); - apr_thread_mutex_unlock(timeout_mutex); if (rc != APR_SUCCESS && !APR_STATUS_IS_EEXIST(rc)) { + TO_QUEUE_REMOVE(cs->sc->wc_q, cs); + apr_thread_mutex_unlock(timeout_mutex); ap_log_error(APLOG_MARK, APLOG_ERR, rc, ap_server_conf, APLOGNO(03465) "process_socket: apr_pollset_add failure for " "write completion"); - apr_thread_mutex_lock(timeout_mutex); - TO_QUEUE_REMOVE(cs->sc->wc_q, cs); - apr_thread_mutex_unlock(timeout_mutex); apr_socket_close(cs->pfd.desc.s); ap_push_pool(worker_queue_info, cs->p); } + else { + apr_thread_mutex_unlock(timeout_mutex); + } return; } else if (c->keepalive != AP_CONN_KEEPALIVE || c->aborted || @@ -1099,18 +1100,17 @@ read_request: apr_thread_mutex_lock(timeout_mutex); TO_QUEUE_APPEND(cs->sc->ka_q, cs); rc = apr_pollset_add(event_pollset, &cs->pfd); - apr_thread_mutex_unlock(timeout_mutex); if (rc != APR_SUCCESS && !APR_STATUS_IS_EEXIST(rc)) { + TO_QUEUE_REMOVE(cs->sc->ka_q, cs); + apr_thread_mutex_unlock(timeout_mutex); ap_log_error(APLOG_MARK, APLOG_ERR, rc, ap_server_conf, APLOGNO(03093) "process_socket: apr_pollset_add failure for " "keep alive"); - apr_thread_mutex_lock(timeout_mutex); - TO_QUEUE_REMOVE(cs->sc->ka_q, cs); - apr_thread_mutex_unlock(timeout_mutex); apr_socket_close(cs->pfd.desc.s); ap_push_pool(worker_queue_info, cs->p); return; } + apr_thread_mutex_unlock(timeout_mutex); } else if (cs->pub.state == CONN_STATE_SUSPENDED) { apr_atomic_inc32(&suspended_count); @@ -1394,7 +1394,6 @@ static void process_lingering_close(event_conn_state_t *cs, const apr_pollfd_t * rv = apr_pollset_remove(event_pollset, pfd); apr_thread_mutex_unlock(timeout_mutex); AP_DEBUG_ASSERT(rv == APR_SUCCESS || APR_STATUS_IS_NOTFOUND(rv)); - TO_QUEUE_ELEM_INIT(cs); rv = apr_socket_close(csd); AP_DEBUG_ASSERT(rv == APR_SUCCESS); @@ -1673,7 +1672,6 @@ static void * APR_THREAD_FUNC listener_thread(apr_thread_t * thd, void *dummy) TO_QUEUE_REMOVE(remove_from_q, cs); rc = apr_pollset_remove(event_pollset, &cs->pfd); apr_thread_mutex_unlock(timeout_mutex); - TO_QUEUE_ELEM_INIT(cs); /* * Some of the pollset backends, like KQueue or Epoll -- 2.40.0