Arrange the bybusyness logic and prevent bad busy values

this closes #383


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

modules/proxy/balancers/mod_lbmethod_bybusyness.c

@@ -15,6 +15,7 @@
  */
 
 #include "mod_proxy.h"
+#include "proxy_util.h"
 #include "scoreboard.h"
 #include "ap_mpm.h"
 #include "apr_version.h"
@@ -25,18 +26,24 @@ module AP_MODULE_DECLARE_DATA lbmethod_bybusyness_module;
 static APR_OPTIONAL_FN_TYPE(proxy_balancer_get_best_worker)
                             *ap_proxy_balancer_get_best_worker_fn = NULL;
 
+
 static int is_best_bybusyness(proxy_worker *current, proxy_worker *prev_best, void *baton)
 {
     int *total_factor = (int *)baton;
+    apr_size_t current_busy = getbusy_count(current);
+    apr_size_t prev_best_busy = 0;
 
     current->s->lbstatus += current->s->lbfactor;
     *total_factor += current->s->lbfactor;
+    if (prev_best)
+        prev_best_busy = getbusy_count(prev_best);
+     
 
     return (
         !prev_best
-        || (current->s->busy < prev_best->s->busy)
+        || (current_busy < prev_best_busy)
         || (
-            (current->s->busy == prev_best->s->busy)
+            (current_busy == prev_best_busy)
             && (current->s->lbstatus > prev_best->s->lbstatus)
         )
     );
@@ -65,7 +72,7 @@ static apr_status_t reset(proxy_balancer *balancer, server_rec *s)
     worker = (proxy_worker **)balancer->workers->elts;
     for (i = 0; i < balancer->workers->nelts; i++, worker++) {
         (*worker)->s->lbstatus = 0;
-        (*worker)->s->busy = 0;
+        setbusy_count(*worker, 0); 
     }
     return APR_SUCCESS;
 }

modules/proxy/mod_proxy_balancer.c

@@ -17,6 +17,7 @@
 /* Load balancer module for Apache proxy */
 
 #include "mod_proxy.h"
+#include "proxy_util.h"
 #include "scoreboard.h"
 #include "ap_mpm.h"
 #include "apr_version.h"
@@ -486,17 +487,6 @@ static void force_recovery(proxy_balancer *balancer, server_rec *s)
     }
 }
 
-static apr_status_t decrement_busy_count(void *worker_)
-{
-    proxy_worker *worker = worker_;
-    
-    if (worker->s->busy) {
-        worker->s->busy--;
-    }
-
-    return APR_SUCCESS;
-}
-
 static int proxy_balancer_pre_request(proxy_worker **worker,
                                       proxy_balancer **balancer,
                                       request_rec *r,
@@ -635,7 +625,7 @@ static int proxy_balancer_pre_request(proxy_worker **worker,
         *worker = runtime;
     }
 
-    (*worker)->s->busy++;
+    increment_busy_count(*worker);
     apr_pool_cleanup_register(r->pool, *worker, decrement_busy_count,
                               apr_pool_cleanup_null);
 
@@ -1575,7 +1565,7 @@ static void balancer_display_page(request_rec *r, proxy_server_conf *conf,
                           "</httpd:redirect>\n", NULL);
                 ap_rprintf(r,
                            "          <httpd:busy>%" APR_SIZE_T_FMT "</httpd:busy>\n",
-                           worker->s->busy);
+                           getbusy_count(worker));
                 ap_rprintf(r, "          <httpd:lbset>%d</httpd:lbset>\n",
                            worker->s->lbset);
                 /* End proxy_worker_stat */
@@ -1748,7 +1738,7 @@ static void balancer_display_page(request_rec *r, proxy_server_conf *conf,
                 ap_rvputs(r, ap_proxy_parse_wstatus(r->pool, worker), NULL);
                 ap_rputs("</td>", r);
                 ap_rprintf(r, "<td>%" APR_SIZE_T_FMT "</td>", worker->s->elected);
-                ap_rprintf(r, "<td>%" APR_SIZE_T_FMT "</td>", worker->s->busy);
+                ap_rprintf(r, "<td>%" APR_SIZE_T_FMT "</td>", getbusy_count(worker));
                 ap_rprintf(r, "<td>%d</td><td>", worker->s->lbstatus);
                 ap_rputs(apr_strfsize(worker->s->transferred, fbuf), r);
                 ap_rputs("</td><td>", r);

modules/proxy/proxy_util.c

@@ -21,6 +21,7 @@
 #include "apr_version.h"
 #include "apr_strings.h"
 #include "apr_hash.h"
+#include "apr_atomic.h"
 #include "http_core.h"
 #include "proxy_util.h"
 #include "ajp.h"
@@ -4984,6 +4985,124 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tunnel_create(proxy_tunnel_rec **ptunnel,
     return APR_SUCCESS;
 }
 
+PROXY_DECLARE(apr_status_t) decrement_busy_count(void *worker_)
+{
+    apr_size_t val;
+    proxy_worker *worker = worker_;
+
+#if APR_SIZEOF_VOIDP == 4
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t));
+    val = apr_atomic_read32(&worker->s->busy);
+    while (val > 0) {
+        apr_size_t old = val;
+        val = apr_atomic_cas32(&worker->s->busy, val - 1, old);
+        if (val == old) {
+            break;
+        }
+    }
+#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t));
+    val = apr_atomic_read64(&worker->s->busy);
+    while (val > 0) {
+        apr_size_t old = val;
+        val = apr_atomic_cas64(&worker->s->busy, val - 1, old);
+        if (val == old) {
+            break;
+        }
+    }
+#else /* Use atomics for (64bit) pointers */
+    void *volatile *busy_p = (void *)&worker->s->busy;
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*));
+    AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0);
+    val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, NULL, NULL);
+    while (val > 0) {
+        apr_size_t old = val;
+        val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p,
+                                               (void *)(apr_uintptr_t)(val - 1),
+                                               (void *)(apr_uintptr_t)old);
+        if (val == old) {
+            break;
+        }
+    }
+#endif
+    return APR_SUCCESS;
+}
+
+PROXY_DECLARE(void) increment_busy_count(proxy_worker *worker)
+{
+    apr_size_t val;
+#if APR_SIZEOF_VOIDP == 4
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t));
+    val = apr_atomic_read32(&worker->s->busy);
+    while (val < APR_INT32_MAX) {
+        apr_size_t old = val;
+        val = apr_atomic_cas32(&worker->s->busy, val + 1, old);
+        if (val == old) {
+            break;
+        }
+    }
+#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t));
+    val = apr_atomic_read64(&worker->s->busy);
+    while (val < APR_INT64_MAX) {
+        apr_size_t old = val;
+        val = apr_atomic_cas64(&worker->s->busy, val + 1, old);
+        if (val == old) {
+            break;
+        }
+    }
+#else /* Use atomics for (64bit) pointers */
+    void *volatile *busy_p = (void *)&worker->s->busy;
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*));
+    AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0);
+    val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, NULL, NULL);
+    while (val < APR_INT64_MAX) {
+        apr_size_t old = val;
+        val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p,
+                                               (void *)(apr_uintptr_t)(val + 1),
+                                               (void *)(apr_uintptr_t)old);
+        if (val == old) {
+            break;
+        }
+    }
+#endif
+}
+
+PROXY_DECLARE(apr_size_t) getbusy_count(proxy_worker *worker)
+{
+    apr_size_t val;
+#if APR_SIZEOF_VOIDP == 4
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t));
+    val = apr_atomic_read32(&worker->s->busy);
+#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t));
+    val = apr_atomic_read64(&worker->s->busy);
+#else /* Use atomics for (64bit) pointers */
+    void *volatile *busy_p = (void *)&worker->s->busy;
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*));
+    AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0);
+    val = (apr_uintptr_t)apr_atomic_casptr((void *)busy_p, NULL, NULL);
+#endif
+
+    return val;
+}
+
+PROXY_DECLARE(void) setbusy_count(proxy_worker *worker, apr_size_t to)
+{
+#if APR_SIZEOF_VOIDP == 4
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint32_t));
+    apr_atomic_set32(&worker->s->busy, to);
+#elif APR_VERSION_AT_LEAST(1,7,4) /* APR 64bit atomics not safe before 1.7.4 */
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(apr_uint64_t));
+    apr_atomic_set64(&worker->s->busy, to);
+#else /* Use atomics for (64bit) pointers */
+    void *volatile *busy_p = (void *)&worker->s->busy;
+    AP_DEBUG_ASSERT(sizeof(apr_size_t) == sizeof(void*));
+    AP_DEBUG_ASSERT((apr_uintptr_t)busy_p % sizeof(void*) == 0);
+    apr_atomic_xchgptr((void *)busy_p, (void *)(apr_uintptr_t)to);
+#endif
+}
+
 static void add_pollset(apr_pollset_t *pollset, apr_pollfd_t *pfd,
                         apr_int16_t events)
 {

modules/proxy/proxy_util.h

@@ -40,6 +40,41 @@ extern PROXY_DECLARE_DATA const apr_strmatch_pattern *ap_proxy_strmatch_domain;
  */
 void proxy_util_register_hooks(apr_pool_t *p);
 
+/*
+ * Get the busy counter from the shared worker memory
+ *
+ * @param worker Pointer to the worker structure.
+ * @return      apr_size_t value atomically read for the worker.
+ */
+PROXY_DECLARE(apr_size_t) getbusy_count(proxy_worker *worker);
+
+/*
+ * Set the busy counter from the shared worker memory
+ *
+ * @param worker Pointer to the worker structure.
+ * @param to value to set the busy counter.
+ * @return      void
+ */
+PROXY_DECLARE(void) setbusy_count(proxy_worker *worker, apr_size_t to);
+
+/*
+ * decrement the busy counter from the shared worker memory
+ * note it is called by apr_pool_cleanup_register()
+ * therfore the void * and apr_status_t.
+ *
+ * @param worker_ Pointer to the worker structure.
+ * @return      apr_status_t returns APR_SUCCESS.
+ */
+PROXY_DECLARE(apr_status_t) decrement_busy_count(void *worker_);
+
+/*
+ * increment the busy counter from the shared worker memory
+ *
+ * @param worker Pointer to the worker structure.
+ * @return      void
+ */
+PROXY_DECLARE(void) increment_busy_count(proxy_worker *worker);
+
 /** @} */
 
 #endif /* PROXY_UTIL_H_ */