You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ji...@apache.org on 2012/07/23 14:35:23 UTC

svn commit: r1364613 - in /httpd/httpd/branches/2.4.x: CHANGES STATUS include/httpd.h server/mpm/event/event.c

Author: jim
Date: Mon Jul 23 12:35:23 2012
New Revision: 1364613

URL: http://svn.apache.org/viewvc?rev=1364613&view=rev
Log:
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

Modified:
    httpd/httpd/branches/2.4.x/CHANGES
    httpd/httpd/branches/2.4.x/STATUS
    httpd/httpd/branches/2.4.x/include/httpd.h
    httpd/httpd/branches/2.4.x/server/mpm/event/event.c

Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1364613&r1=1364612&r2=1364613&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Mon Jul 23 12:35:23 2012
@@ -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 <heinenn google.com>]
 
+  *) 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]

Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1364613&r1=1364612&r2=1364613&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Mon Jul 23 12:35:23 2012
@@ -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:

Modified: httpd/httpd/branches/2.4.x/include/httpd.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/include/httpd.h?rev=1364613&r1=1364612&r2=1364613&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/include/httpd.h (original)
+++ httpd/httpd/branches/2.4.x/include/httpd.h Mon Jul 23 12:35:23 2012
@@ -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;
 
 /**

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=1364613&r1=1364612&r2=1364613&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 Mon Jul 23 12:35:23 2012
@@ -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_t
                         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_t
                      * 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_t
                              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 */