Mailing-List: contact cvs-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1343087 - in /httpd/httpd/trunk: CHANGES
 server/mpm/event/event.c
Date: Sun, 27 May 2012 20:23:43 -0000
To: cvs@httpd.apache.org
From: sf@apache.org
Message-Id: <20120527202343.E7198238896F@eris.apache.org>

Author: sf
Date: Sun May 27 20:23:43 2012
New Revision: 1343087

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

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/server/mpm/event/event.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1343087&r1=1343086&r2=1343087&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sun May 27 20:23:43 2012
@@ -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]

Modified: httpd/httpd/trunk/server/mpm/event/event.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1343087&r1=1343086&r2=1343087&view=diff
==============================================================================
--- httpd/httpd/trunk/server/mpm/event/event.c (original)
+++ httpd/httpd/trunk/server/mpm/event/event.c Sun May 27 20:23:43 2012
@@ -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_co
 
 /*
  * 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 *
             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_t
                 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_thre
         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 */