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

diff --git a/CHANGES b/CHANGES
index c71b04bf568bb7c4156036f4048b0775aab5fddf..3a0f27bafdc4e1db10ce6575592c744bde686cc4 100644 (file)
--- 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 <jose w3.org>, Yann Ylavic]
 

diff --git a/STATUS b/STATUS
index 87bafa7c80f1e7b22971ee77ead1aa7721980056..9ff8e1d869e7140677f62c087492a48b7b6606d0 100644 (file)
--- 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 891ed3b313075785a963f320e65351442ff933f3..bd3ad3c9989d10ee18f7640ea914a27c46b87030 100644 (file)
--- 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