From: Stefan Fritsch <sf@apache.org>
Date: Sun, 27 May 2012 20:23:43 +0000 (+0000)
Subject: Fix MaxConnectionsPerChild
X-Git-Tag: 2.5.0-alpha~6786
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c30fd06363d12467013caa6c8b886dd815eb09f8;p=thirdparty%2Fapache%2Fhttpd.git

Fix MaxConnectionsPerChild

This was broken when the handling of lingering close was moved into the
listener thread.

- Make the connection counting thread safe.
- Do the counting in the connection pool cleanup to ensure that it gets
  also executed if the listener thread closes the connection.
- Add a trace log message when a process is recycled.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1343087 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/CHANGES b/CHANGES
index 7b805b79b14..34f7f5e17f8 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mpm_event: Fix handling of MaxConnectionsPerChild. [Stefan Fritsch]
+
   *) suexec: Add --enable-suexec-capabilites support on Linux, to use
      setuid/setgid capability bits rather than a setuid root binary.
      [Joe Orton]
diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c
index a12adba3f93..f228a25db58 100644
--- a/server/mpm/event/event.c
+++ b/server/mpm/event/event.c
@@ -175,8 +175,14 @@ static int dying = 0;
 static int workers_may_exit = 0;
 static int start_thread_may_exit = 0;
 static int listener_may_exit = 0;
-static int conns_this_child;
 static int num_listensocks = 0;
+/*
+ * As we don't have APR atomic functions for apr_int32_t, we use apr_uint32_t
+ * to actually store a signed value. This works as long as the platform uses
+ * two's complement representation. This assumption is also made in other
+ * parts of the code (e.g. fdqueue.c).
+ */
+static apr_uint32_t conns_this_child;
 static apr_uint32_t connection_count = 0;
 static int resource_shortage = 0;
 static fd_queue_t *worker_queue;
@@ -609,6 +615,7 @@ static int volatile restart_pending;
 
 static apr_status_t decrement_connection_count(void *dummy) {
     apr_atomic_dec32(&connection_count);
+    apr_atomic_dec32(&conns_this_child);
     return APR_SUCCESS;
 }
 
@@ -871,10 +878,8 @@ static int stop_lingering_close(event_conn_state_t *cs)
 
 /*
  * process one connection in the worker
- * return: 1 if the connection has been completed,
- *         0 if it is still open and waiting for some event
  */
-static int process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * sock,
+static void process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * sock,
                           event_conn_state_t * cs, int my_child_num,
                           int my_thread_num)
 {
@@ -896,7 +901,8 @@ static int process_socket(apr_thread_t *thd, apr_pool_t * p, apr_socket_t * sock
             apr_bucket_alloc_destroy(cs->bucket_alloc);
             apr_pool_clear(p);
             ap_push_pool(worker_queue_info, p);
-            return 1;
+            apr_atomic_dec32(&conns_this_child);
+            return;
         }
         apr_atomic_inc32(&connection_count);
         apr_pool_cleanup_register(c->pool, NULL, decrement_connection_count, apr_pool_cleanup_null);
@@ -993,7 +999,7 @@ read_request:
             cs->pfd.reqevents = APR_POLLOUT | APR_POLLHUP | APR_POLLERR;
             rc = apr_pollset_add(event_pollset, &cs->pfd);
             apr_thread_mutex_unlock(timeout_mutex);
-            return 1;
+            return;
         }
         else if (c->keepalive != AP_CONN_KEEPALIVE || c->aborted ||
             listener_may_exit) {
@@ -1010,7 +1016,7 @@ read_request:
 
     if (cs->pub.state == CONN_STATE_LINGER) {
         if (!start_lingering_close_blocking(cs))
-            return 0;
+            return;
     }
     else if (cs->pub.state == CONN_STATE_CHECK_REQUEST_LINE_READABLE) {
         apr_status_t rc;
@@ -1046,7 +1052,7 @@ read_request:
      * or timeout.
      */
     c->sbh = NULL;
-    return 1;
+    return;
 }
 
 /* conns_this_child has gone to zero or below.  See if the admin coded
@@ -1055,10 +1061,13 @@ read_request:
 static void check_infinite_requests(void)
 {
     if (ap_max_requests_per_child) {
+        ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, ap_server_conf,
+                     "Stopping process due to MaxConnectionsPerChild");
         signal_threads(ST_GRACEFUL);
     }
     else {
-        conns_this_child = INT_MAX;  /* keep going */
+        /* keep going */
+        apr_atomic_set32(&conns_this_child, APR_INT32_MAX);
     }
 }
 
@@ -1447,7 +1456,7 @@ static void * APR_THREAD_FUNC listener_thread(apr_thread_t * thd, void *dummy)
                 break;
         }
 
-        if (conns_this_child <= 0) {
+        if (((apr_int32_t)apr_atomic_read32(&conns_this_child)) <= 0) {
             check_infinite_requests();
         }
 
@@ -1848,10 +1857,7 @@ static void *APR_THREAD_FUNC worker_thread(apr_thread_t * thd, void *dummy)
         else {
             is_idle = 0;
             worker_sockets[thread_slot] = csd;
-            rv = process_socket(thd, ptrans, csd, cs, process_slot, thread_slot);
-            if (!rv) {
-                conns_this_child--;
-            }
+            process_socket(thd, ptrans, csd, cs, process_slot, thread_slot);
             worker_sockets[thread_slot] = NULL;
         }
     }
@@ -2148,11 +2154,11 @@ static void child_main(int child_num_arg)
     }
 
     if (ap_max_requests_per_child) {
-        conns_this_child = ap_max_requests_per_child;
+        apr_atomic_set32(&conns_this_child, (apr_uint32_t)ap_max_requests_per_child);
     }
     else {
         /* coding a value of zero means infinity */
-        conns_this_child = INT_MAX;
+        apr_atomic_set32(&conns_this_child, APR_INT32_MAX);
     }
 
     /* Setup worker threads */