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

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

diff --git a/CHANGES b/CHANGES
index 4b1efaf6a608575bb03259d69054858466aaacb0..3cb2c9eebc2b375d161dc702fa07954be8c43d4a 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -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]

diff --git a/STATUS b/STATUS
index 7d1476783813b9d626736118b15b73858c01060b..2ad56780f144b35fbcbe87321836b89cc10e19c1 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -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:

diff --git a/include/httpd.h b/include/httpd.h
index c5f3872ac73ca0a520b23a6e6013cbf32926ff1b..e718bfc7c1161ae2ae448236d8c4c6b15fb28b32 100644 (file)
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -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;
 
 /**

diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c
index 007d887f7d6794e80eab80c891031f0b0a67900f..0934e0dc0c96bd7f8c632696e5f767048cb96ba8 100644 (file)
--- a/server/mpm/event/event.c
+++ b/server/mpm/event/event.c
@@ -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_thread(apr_thread_t * thd, void *dummy)
                         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_thread(apr_thread_t * thd, void *dummy)
                      * 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_thread(apr_thread_t * thd, void *dummy)
                              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 */