clean up the fdqueue a bit more, by implementing Greg's

suggestions to getting rid of redundant variables, as well as
converting the simple ap_queue_full/ap_queue_empty tests into macros.
This also reinstates the "not_full" condition, which turned out to
be useful afterall in cases where we wanted to prevent the listener
thread from accepting further connections until the worker queue has
room for more.

Submitted by:	[Aaron Bannert <aaron@clove.org>]

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

diff --git a/server/mpm/worker/fdqueue.c b/server/mpm/worker/fdqueue.c
index 753e6ed1ecb63bd131063f76b7cdb40d1fd0e621..b0e20540890eab31f094492daa2bffa8c889cb7c 100644 (file)
--- a/server/mpm/worker/fdqueue.c
+++ b/server/mpm/worker/fdqueue.c
@@ -62,19 +62,13 @@
  * Detects when the fd_queue_t is full. This utility function is expected
  * to be called from within critical sections, and is not threadsafe.
  */
-static int ap_queue_full(fd_queue_t *queue)
-{
-    return (queue->blanks <= 0);
-}
+#define ap_queue_full(queue) ((queue)->tail == (queue)->bounds)
 
 /**
  * Detects when the fd_queue_t is empty. This utility function is expected
  * to be called from within critical sections, and is not threadsafe.
  */
-static int ap_queue_empty(fd_queue_t *queue)
-{
-    return (queue->blanks >= queue->bounds - 1);
-}
+#define ap_queue_empty(queue) ((queue)->tail == 0)
 
 /**
  * Callback routine that is called to destroy this
@@ -88,6 +82,7 @@ static apr_status_t ap_queue_destroy(void *data)
      * XXX: We should at least try to signal an error here, it is
      * indicative of a programmer error. -aaron */
     pthread_cond_destroy(&queue->not_empty);
+    pthread_cond_destroy(&queue->not_full);
     pthread_mutex_destroy(&queue->one_big_mutex);
 
     return FD_QUEUE_SUCCESS;
@@ -99,21 +94,20 @@ static apr_status_t ap_queue_destroy(void *data)
 int ap_queue_init(fd_queue_t *queue, int queue_capacity, apr_pool_t *a) 
 {
     int i;
-    int bounds;
 
     if (pthread_mutex_init(&queue->one_big_mutex, NULL) != 0)
         return FD_QUEUE_FAILURE;
     if (pthread_cond_init(&queue->not_empty, NULL) != 0)
         return FD_QUEUE_FAILURE;
+    if (pthread_cond_init(&queue->not_full, NULL) != 0)
+        return FD_QUEUE_FAILURE;
 
-    bounds = queue_capacity + 1;
     queue->tail = 0;
-    queue->data = apr_palloc(a, bounds * sizeof(fd_queue_elem_t));
-    queue->bounds = bounds;
-    queue->blanks = queue_capacity;
+    queue->data = apr_palloc(a, queue_capacity * sizeof(fd_queue_elem_t));
+    queue->bounds = queue_capacity;
 
     /* Set all the sockets in the queue to NULL */
-    for (i = 0; i < bounds; ++i)
+    for (i = 0; i < queue_capacity; ++i)
         queue->data[i].sd = NULL;
 
     apr_pool_cleanup_register(a, queue, ap_queue_destroy, apr_pool_cleanup_null);
@@ -128,23 +122,19 @@ int ap_queue_init(fd_queue_t *queue, int queue_capacity, apr_pool_t *a)
  */
 int ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p) 
 {
+    fd_queue_elem_t *elem;
+
     if (pthread_mutex_lock(&queue->one_big_mutex) != 0) {
         return FD_QUEUE_FAILURE;
     }
 
-    /* If the caller didn't allocate enough slots and tries to push
-     * too many, too bad. */
-    if (ap_queue_full(queue)) {
-        if (pthread_mutex_unlock(&queue->one_big_mutex) != 0) {
-            return FD_QUEUE_FAILURE;
-        }
-        return FD_QUEUE_OVERFLOW;
+    while (ap_queue_full(queue)) {
+        pthread_cond_wait(&queue->not_full, &queue->one_big_mutex);
     }
 
-    queue->data[queue->tail].sd = sd;
-    queue->data[queue->tail].p = p;
-    queue->tail++;
-    queue->blanks--;
+    elem = &queue->data[queue->tail++];
+    elem->sd = sd;
+    elem->p = p;
 
     pthread_cond_signal(&queue->not_empty);
 
@@ -181,13 +171,16 @@ apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p)
         }
     } 
     
-    queue->tail--;
-    elem = &queue->data[queue->tail];
+    elem = &queue->data[--queue->tail];
     *sd = elem->sd;
     *p = elem->p;
     elem->sd = NULL;
     elem->p = NULL;
-    queue->blanks++;
+
+    /* signal not_full if we were full before this pop */
+    if (queue->tail == queue->bounds - 1) {
+        pthread_cond_signal(&queue->not_full);
+    }
 
     if (pthread_mutex_unlock(&queue->one_big_mutex) != 0) {
         return FD_QUEUE_FAILURE;
@@ -202,6 +195,9 @@ apr_status_t ap_queue_interrupt_all(fd_queue_t *queue)
         return FD_QUEUE_FAILURE;
     }
     pthread_cond_broadcast(&queue->not_empty);
+    /* We shouldn't have multiple threads sitting in not_full, but
+     * broadcast just in case. */
+    pthread_cond_broadcast(&queue->not_full);
     if (pthread_mutex_unlock(&queue->one_big_mutex) != 0) {
         return FD_QUEUE_FAILURE;
     }

diff --git a/server/mpm/worker/fdqueue.h b/server/mpm/worker/fdqueue.h
index 7ac951258b2da2aa0d4f32c4e651bbd337ac516e..5b8aea05174fdf1817c3aa52c91959ac2942becc 100644 (file)
--- a/server/mpm/worker/fdqueue.h
+++ b/server/mpm/worker/fdqueue.h
@@ -72,7 +72,6 @@
 #define FD_QUEUE_FAILURE -1 /* Needs to be an invalid file descriptor because
                                of queue_pop semantics */
 #define FD_QUEUE_EINTR APR_EINTR
-#define FD_QUEUE_OVERFLOW -2
 
 struct fd_queue_elem_t {
     apr_socket_t      *sd;
@@ -87,6 +86,7 @@ struct fd_queue_t {
     int                blanks;
     pthread_mutex_t    one_big_mutex;
     pthread_cond_t     not_empty;
+    pthread_cond_t     not_full;
     int                cancel_state;
 };
 typedef struct fd_queue_t fd_queue_t;