Convert the ap_queue_foo routines to return apr_status_t as appropriate.

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

diff --git a/CHANGES b/CHANGES
index 4efe28373eee4a03af9bd88881475883ff231fe2..3137ee7ac44979a16e4e189ba34ac44414a8acc4 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,8 @@
 Changes with Apache 2.0.33-dev
 
+  *) worker MPM: Improve logging of errors with the interface between
+     the listener thread and worker threads.  [Jeff Trawick]
+
   *) Some browsers ignore cookies that have been merged into a
      single Set-Cookie header. Set-Cookie and Set-Cookie2 headers
      are now unmerged in the http proxy before being sent to the

diff --git a/server/mpm/worker/fdqueue.c b/server/mpm/worker/fdqueue.c
index 338588e0492c785f6adbc454a24f0236b2605172..70671d0d3c51e657c7289dda08d4c8987a5f12ec 100644 (file)
--- a/server/mpm/worker/fdqueue.c
+++ b/server/mpm/worker/fdqueue.c
@@ -85,24 +85,27 @@ static apr_status_t ap_queue_destroy(void *data)
     apr_thread_cond_destroy(queue->not_full);
     apr_thread_mutex_destroy(queue->one_big_mutex);
 
-    return FD_QUEUE_SUCCESS;
+    return APR_SUCCESS;
 }
 
 /**
  * Initialize the fd_queue_t.
  */
-int ap_queue_init(fd_queue_t *queue, int queue_capacity, apr_pool_t *a) 
+apr_status_t ap_queue_init(fd_queue_t *queue, int queue_capacity, apr_pool_t *a)
 {
     int i;
+    apr_status_t rv;
 
-    /* FIXME: APRize these return values. */
-    if (apr_thread_mutex_create(&queue->one_big_mutex,
-                              APR_THREAD_MUTEX_DEFAULT, a) != APR_SUCCESS)
-        return FD_QUEUE_FAILURE;
-    if (apr_thread_cond_create(&queue->not_empty, a) != APR_SUCCESS)
-        return FD_QUEUE_FAILURE;
-    if (apr_thread_cond_create(&queue->not_full, a) != APR_SUCCESS)
-        return FD_QUEUE_FAILURE;
+    if ((rv = apr_thread_mutex_create(&queue->one_big_mutex,
+                                      APR_THREAD_MUTEX_DEFAULT, a)) != APR_SUCCESS) {
+        return rv;
+    }
+    if ((rv = apr_thread_cond_create(&queue->not_empty, a)) != APR_SUCCESS) {
+        return rv;
+    }
+    if ((rv = apr_thread_cond_create(&queue->not_full, a)) != APR_SUCCESS) {
+        return rv;
+    }
 
     queue->tail = 0;
     queue->data = apr_palloc(a, queue_capacity * sizeof(fd_queue_elem_t));
@@ -118,7 +121,7 @@ int ap_queue_init(fd_queue_t *queue, int queue_capacity, apr_pool_t *a)
 
     apr_pool_cleanup_register(a, queue, ap_queue_destroy, apr_pool_cleanup_null);
 
-    return FD_QUEUE_SUCCESS;
+    return APR_SUCCESS;
 }
 
 /**
@@ -126,14 +129,15 @@ int ap_queue_init(fd_queue_t *queue, int queue_capacity, apr_pool_t *a)
  * the push operation has completed, it signals other threads waiting
  * in apr_queue_pop() that they may continue consuming sockets.
  */
-int ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p,
-                  apr_pool_t **recycled_pool)
+apr_status_t ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p,
+                           apr_pool_t **recycled_pool)
 {
     fd_queue_elem_t *elem;
+    apr_status_t rv;
 
     *recycled_pool = NULL;
-    if (apr_thread_mutex_lock(queue->one_big_mutex) != APR_SUCCESS) {
-        return FD_QUEUE_FAILURE;
+    if ((rv = apr_thread_mutex_lock(queue->one_big_mutex)) != APR_SUCCESS) {
+        return rv;
     }
 
     while (ap_queue_full(queue)) {
@@ -150,11 +154,11 @@ int ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p,
 
     apr_thread_cond_signal(queue->not_empty);
 
-    if (apr_thread_mutex_unlock(queue->one_big_mutex) != APR_SUCCESS) {
-        return FD_QUEUE_FAILURE;
+    if ((rv = apr_thread_mutex_unlock(queue->one_big_mutex)) != APR_SUCCESS) {
+        return rv;
     }
 
-    return FD_QUEUE_SUCCESS;
+    return APR_SUCCESS;
 }
 
 /**
@@ -167,12 +171,13 @@ apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p,
                           apr_pool_t *recycled_pool) 
 {
     fd_queue_elem_t *elem;
+    apr_status_t rv;
 
-    if (apr_thread_mutex_lock(queue->one_big_mutex) != APR_SUCCESS) {
+    if ((rv = apr_thread_mutex_lock(queue->one_big_mutex)) != APR_SUCCESS) {
         if (recycled_pool) {
             apr_pool_destroy(recycled_pool);
         }
-        return FD_QUEUE_FAILURE;
+        return rv;
     }
 
     if (recycled_pool) {
@@ -189,10 +194,10 @@ apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p,
         apr_thread_cond_wait(queue->not_empty, queue->one_big_mutex);
         /* If we wake up and it's still empty, then we were interrupted */
         if (ap_queue_empty(queue)) {
-            if (apr_thread_mutex_unlock(queue->one_big_mutex) != APR_SUCCESS) {
-                return FD_QUEUE_FAILURE;
+            if ((rv = apr_thread_mutex_unlock(queue->one_big_mutex)) != APR_SUCCESS) {
+                return rv;
             }
-            return FD_QUEUE_EINTR;
+            return APR_EINTR;
         }
     } 
     
@@ -207,8 +212,8 @@ apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p,
         apr_thread_cond_signal(queue->not_full);
     }
 
-    if (apr_thread_mutex_unlock(queue->one_big_mutex) != APR_SUCCESS) {
-        return FD_QUEUE_FAILURE;
+    if ((rv = apr_thread_mutex_unlock(queue->one_big_mutex)) != APR_SUCCESS) {
+        return rv;
     }
 
     return APR_SUCCESS;
@@ -216,16 +221,18 @@ apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p,
 
 apr_status_t ap_queue_interrupt_all(fd_queue_t *queue)
 {
-    if (apr_thread_mutex_lock(queue->one_big_mutex) != APR_SUCCESS) {
-        return FD_QUEUE_FAILURE;
+    apr_status_t rv;
+    
+    if ((rv = apr_thread_mutex_lock(queue->one_big_mutex)) != APR_SUCCESS) {
+        return rv;
     }
     apr_thread_cond_broadcast(queue->not_empty);
     /* We shouldn't have multiple threads sitting in not_full, but
      * broadcast just in case. */
     apr_thread_cond_broadcast(queue->not_full);
-    if (apr_thread_mutex_unlock(queue->one_big_mutex) != APR_SUCCESS) {
-        return FD_QUEUE_FAILURE;
+    if ((rv = apr_thread_mutex_unlock(queue->one_big_mutex)) != APR_SUCCESS) {
+        return rv;
     }
-    return FD_QUEUE_SUCCESS;
+    return APR_SUCCESS;
 }
 

diff --git a/server/mpm/worker/fdqueue.h b/server/mpm/worker/fdqueue.h
index ad0064a741dac3f4ad7fc854957802b655db3ff6..0c2adcd05054b1c3c77bf205453a9add36e17335 100644 (file)
--- a/server/mpm/worker/fdqueue.h
+++ b/server/mpm/worker/fdqueue.h
@@ -71,11 +71,6 @@
 #endif
 #include <apr_errno.h>
 
-#define FD_QUEUE_SUCCESS 0
-#define FD_QUEUE_FAILURE -1 /* Needs to be an invalid file descriptor because
-                               of queue_pop semantics */
-#define FD_QUEUE_EINTR APR_EINTR
-
 struct fd_queue_elem_t {
     apr_socket_t      *sd;
     apr_pool_t        *p;
@@ -96,10 +91,9 @@ struct fd_queue_t {
 };
 typedef struct fd_queue_t fd_queue_t;
 
-/* FIXME: APRize these -- return values should be apr_status_t */
-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,
-                  apr_pool_t **recycled_pool);
+apr_status_t ap_queue_init(fd_queue_t *queue, int queue_capacity, apr_pool_t *a);
+apr_status_t ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p,
+                           apr_pool_t **recycled_pool);
 apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p,
                           apr_pool_t *recycled_pool);
 apr_status_t ap_queue_interrupt_all(fd_queue_t *queue);

diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c
index b2b2f2662b5de38ff719e2b40b000eac235ab564..309c0aac9b93e11cb006d99c67c9907df8baf83e 100644 (file)
--- a/server/mpm/worker/worker.c
+++ b/server/mpm/worker/worker.c
@@ -706,9 +706,8 @@ static void *listener_thread(apr_thread_t *thd, void * dummy)
                      * socket to a worker 
                      */
                     apr_socket_close(csd);
-                    ap_log_error(APLOG_MARK, APLOG_CRIT, 0, ap_server_conf,
-                                 "ap_queue_push failed with error code %d",
-                                 rv);
+                    ap_log_error(APLOG_MARK, APLOG_CRIT, rv, ap_server_conf,
+                                 "ap_queue_push failed");
                 }
             }
         }
@@ -753,11 +752,11 @@ static void * APR_THREAD_FUNC worker_thread(apr_thread_t *thd, void * dummy)
         rv = ap_queue_pop(worker_queue, &csd, &ptrans, last_ptrans);
         last_ptrans = NULL;
 
-        /* We get FD_QUEUE_EINTR whenever ap_queue_pop() has been interrupted
+        /* We get APR_EINTR whenever ap_queue_pop() has been interrupted
          * from an explicit call to ap_queue_interrupt_all(). This allows
          * us to unblock threads stuck in ap_queue_pop() when a shutdown
          * is pending. */
-        if (rv == FD_QUEUE_EINTR || !csd) {
+        if (rv == APR_EINTR || !csd) {
             continue;
         }
         process_socket(ptrans, csd, process_slot, thread_slot);
@@ -802,7 +801,12 @@ static void * APR_THREAD_FUNC start_threads(apr_thread_t *thd, void *dummy)
     /* We must create the fd queues before we start up the listener
      * and worker threads. */
     worker_queue = apr_pcalloc(pchild, sizeof(*worker_queue));
-    ap_queue_init(worker_queue, ap_threads_per_child, pchild);
+    rv = ap_queue_init(worker_queue, ap_threads_per_child, pchild);
+    if (rv != APR_SUCCESS) {
+        ap_log_error(APLOG_MARK, APLOG_ALERT, rv, ap_server_conf,
+                     "ap_queue_init() failed");
+        clean_child_exit(APEXIT_CHILDFATAL);
+    }
 
     my_info = (proc_info *)malloc(sizeof(proc_info));
     my_info->pid = my_child_num;
@@ -815,7 +819,10 @@ static void * APR_THREAD_FUNC start_threads(apr_thread_t *thd, void *dummy)
                      "apr_thread_create: unable to create worker thread");
         /* In case system resources are maxxed out, we don't want
          * Apache running away with the CPU trying to fork over and
-         * over and over again if we exit. */
+         * over and over again if we exit.
+         * XXX Jeff doesn't see how Apache is going to try to fork again since
+         * the exit code is APEXIT_CHILDFATAL
+         */
         apr_sleep(10 * APR_USEC_PER_SEC);
         clean_child_exit(APEXIT_CHILDFATAL);
     }