Merge r1639614 from trunk:

don't call notify_suspend() in a worker thread after
start_lingering_close_common may have put the socket back
into the pollset.

If it becomes readable too quickly, cs can be
free'ed or accessed concurrently.

Submitted by: covener
Reviewed/backported by: jim

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

diff --git a/STATUS b/STATUS
index c7ac098f7b6bd496cdea879d2751011d4727dd5d..9a6a882dee80423e188e1ac033834c82b3c71be8 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -119,12 +119,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
      2.4.x patch: trunk works
      +1 covener, ylavic, jim
 
-    * event: avoid potentially concurrent access to a 'cs' right as it goes
-      into lingering close.
-      trunk patch: http://svn.apache.org/r1639614
-      2.4.x patch: trunk works
-      +1: covener, ylavic, jim
-
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]

diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c
index 82e7d11c4e345b09c84141d04b17c4ea4e1adfb9..9db1821a896ea1423c4b1456b856cdd436adbb6e 100644 (file)
--- a/server/mpm/event/event.c
+++ b/server/mpm/event/event.c
@@ -767,7 +767,19 @@ static void set_signals(void)
 #endif
 }
 
-static int start_lingering_close_common(event_conn_state_t *cs)
+static void notify_suspend(event_conn_state_t *cs)
+{
+    ap_run_suspend_connection(cs->c, cs->r);
+    cs->suspended = 1;
+}
+
+static void notify_resume(event_conn_state_t *cs)
+{
+    cs->suspended = 0;
+    ap_run_resume_connection(cs->c, cs->r);
+}
+
+static int start_lingering_close_common(event_conn_state_t *cs, int in_worker)
 {
     apr_status_t rv;
     struct timeout_queue *q;
@@ -805,6 +817,9 @@ static int start_lingering_close_common(event_conn_state_t *cs)
             cs->pub.sense == CONN_SENSE_WANT_WRITE ? APR_POLLOUT :
                     APR_POLLIN) | APR_POLLHUP | APR_POLLERR;
     cs->pub.sense = CONN_SENSE_DEFAULT;
+    if (in_worker) { 
+        notify_suspend(cs);
+    }
     rv = apr_pollset_add(event_pollset, &cs->pfd);
     apr_thread_mutex_unlock(timeout_mutex);
     if (rv != APR_SUCCESS && !APR_STATUS_IS_EEXIST(rv)) {
@@ -836,7 +851,7 @@ static int start_lingering_close_blocking(event_conn_state_t *cs)
         ap_push_pool(worker_queue_info, cs->p);
         return 0;
     }
-    return start_lingering_close_common(cs);
+    return start_lingering_close_common(cs, 1);
 }
 
 /*
@@ -861,7 +876,7 @@ static int start_lingering_close_nonblocking(event_conn_state_t *cs)
         ap_push_pool(worker_queue_info, cs->p);
         return 0;
     }
-    return start_lingering_close_common(cs);
+    return start_lingering_close_common(cs, 0);
 }
 
 /*
@@ -886,18 +901,6 @@ static int stop_lingering_close(event_conn_state_t *cs)
     return 0;
 }
 
-static void notify_suspend(event_conn_state_t *cs)
-{
-    ap_run_suspend_connection(cs->c, cs->r);
-    cs->suspended = 1;
-}
-
-static void notify_resume(event_conn_state_t *cs)
-{
-    cs->suspended = 0;
-    ap_run_resume_connection(cs->c, cs->r);
-}
-
 /*
  * This runs before any non-MPM cleanup code on the connection;
  * if the connection is currently suspended as far as modules
@@ -1089,7 +1092,6 @@ read_request:
 
     if (cs->pub.state == CONN_STATE_LINGER) {
         start_lingering_close_blocking(cs);
-        notify_suspend(cs);
     }
     else if (cs->pub.state == CONN_STATE_CHECK_REQUEST_LINE_READABLE) {
         /* It greatly simplifies the logic to use a single timeout value here