svn commit: r1642858 - in /httpd/httpd/branches/2.4.x: ./ STATUS server/mpm/event/event.c

[ji...@apache.org]

Author: jim
Date: Tue Dec  2 12:48:41 2014
New Revision: 1642858

URL: http://svn.apache.org/r1642858
Log:
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

Modified:
    httpd/httpd/branches/2.4.x/   (props changed)
    httpd/httpd/branches/2.4.x/STATUS
    httpd/httpd/branches/2.4.x/server/mpm/event/event.c

Propchange: httpd/httpd/branches/2.4.x/
------------------------------------------------------------------------------
  Merged /httpd/httpd/trunk:r1639614

Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1642858&r1=1642857&r2=1642858&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Tue Dec  2 12:48:41 2014
@@ -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 ]

Modified: httpd/httpd/branches/2.4.x/server/mpm/event/event.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/server/mpm/event/event.c?rev=1642858&r1=1642857&r2=1642858&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/server/mpm/event/event.c (original)
+++ httpd/httpd/branches/2.4.x/server/mpm/event/event.c Tue Dec  2 12:48:41 2014
@@ -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(
             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_blockin
         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_nonbloc
         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_co
     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