Don't drop connections during graceful restart. Previously, worker

threads could exit even though there were connections waiting in the
queue.

Now, for a graceful restart the worker threads won't exit until they
are told that the queue has been drained and no more connections will
ever be added.

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

diff --git a/CHANGES b/CHANGES
index c8a3dad9a98abd6578e53ec7c9429c0568b4a065..625881c9cbc21f6f7164111ebdcd99f85058eef0 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,8 @@
 Changes with Apache 2.0.34-dev
 
+  *) Fix some restart/terminate problems in the worker MPM.  Don't
+     drop connections during graceful restart.  [Jeff Trawick]
+
   *) Change the header merging behaviour in proxy, as some headers
      (like Set-Cookie) cannot be unmerged due to stray commas in
      dates. [Graham Leggett]

diff --git a/server/mpm/worker/fdqueue.c b/server/mpm/worker/fdqueue.c
index 96c3fe350c213c98f482b933c55ee8ecfa7c158c..c1fdef08dd19f6abf672ebca4e6f7c1333b276be 100644 (file)
--- a/server/mpm/worker/fdqueue.c
+++ b/server/mpm/worker/fdqueue.c
@@ -140,6 +140,8 @@ apr_status_t ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p,
         return rv;
     }
 
+    AP_DEBUG_ASSERT(!queue->terminated);
+    
     while (ap_queue_full(queue)) {
         apr_thread_cond_wait(queue->not_full, queue->one_big_mutex);
     }
@@ -191,13 +193,20 @@ apr_status_t ap_queue_pop(fd_queue_t *queue, apr_socket_t **sd, apr_pool_t **p,
 
     /* Keep waiting until we wake up and find that the queue is not empty. */
     if (ap_queue_empty(queue)) {
-        apr_thread_cond_wait(queue->not_empty, queue->one_big_mutex);
+        if (!queue->terminated) {
+            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 ((rv = apr_thread_mutex_unlock(queue->one_big_mutex)) != APR_SUCCESS) {
                 return rv;
             }
-            return APR_EINTR;
+            if (queue->terminated) {
+                return APR_EOF; /* no more elements ever again */
+            }
+            else {
+                return APR_EINTR;
+            }
         }
     } 
     
@@ -236,3 +245,20 @@ apr_status_t ap_queue_interrupt_all(fd_queue_t *queue)
     return APR_SUCCESS;
 }
 
+apr_status_t ap_queue_term(fd_queue_t *queue)
+{
+    apr_status_t rv;
+
+    if ((rv = apr_thread_mutex_lock(queue->one_big_mutex)) != APR_SUCCESS) {
+        return rv;
+    }
+    /* we must hold one_big_mutex when setting this... otherwise,
+     * we could end up setting it and waking everybody up just after a 
+     * would-be popper checks it but right before they block
+     */
+    queue->terminated = 1;
+    if ((rv = apr_thread_mutex_unlock(queue->one_big_mutex)) != APR_SUCCESS) {
+        return rv;
+    }
+    return ap_queue_interrupt_all(queue);
+}

diff --git a/server/mpm/worker/fdqueue.h b/server/mpm/worker/fdqueue.h
index 2f9ec14d55bdf74fb051de1fa816b3a70367b0f7..094cb2b35ad5601ff0bfad6a49f810cc83b21093 100644 (file)
--- a/server/mpm/worker/fdqueue.h
+++ b/server/mpm/worker/fdqueue.h
@@ -86,6 +86,7 @@ struct fd_queue_t {
     apr_thread_cond_t  *not_full;
     apr_pool_t        **recycled_pools;
     int                 num_recycled;
+    int                 terminated;
 };
 typedef struct fd_queue_t fd_queue_t;
 
@@ -95,5 +96,6 @@ apr_status_t ap_queue_push(fd_queue_t *queue, apr_socket_t *sd, apr_pool_t *p,
 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);
+apr_status_t ap_queue_term(fd_queue_t *queue);
 
 #endif /* FDQUEUE_H */

diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c
index e1e60c186248dedc5a317fbfd12ebb031a6e6a82..6208a750a049d7f618ec5e7f5da2d5f3280a489c 100644 (file)
--- a/server/mpm/worker/worker.c
+++ b/server/mpm/worker/worker.c
@@ -267,17 +267,30 @@ static void wakeup_listener(void)
 
 static void signal_threads(int mode)
 {
+    static int prev_mode = 0;
+    
+    if (prev_mode == mode) {
+        return;
+    }
+    prev_mode = mode;
+
     /* in case we weren't called from the listener thread, wake up the
      * listener thread
      */
     wakeup_listener();
 
+    /* for ungraceful termination, let the workers exit now;
+     * for graceful termination, the listener thread will notify the
+     * workers to exit once it has stopped accepting new connections
+     */
+    if (mode == ST_UNGRACEFUL) {
+        workers_may_exit = 1;
+        ap_queue_interrupt_all(worker_queue);
+    }
+
     /* XXX: This will happen naturally on a graceful, and we don't care 
      * otherwise.
     ap_queue_signal_all_wakeup(worker_queue); */
-
-    workers_may_exit = 1;
-    ap_queue_interrupt_all(worker_queue);
 }
 
 AP_DECLARE(apr_status_t) ap_mpm_query(int query_code, int *result)
@@ -798,6 +811,7 @@ static void *listener_thread(apr_thread_t *thd, void * dummy)
     ap_update_child_status_from_indexes(process_slot, thread_slot, 
                                         (dying) ? SERVER_DEAD : SERVER_GRACEFUL,
                                         (request_rec *) NULL);
+    ap_queue_term(worker_queue);
     dying = 1;
     ap_scoreboard_image->parent[process_slot].quiescing = 1;
     kill(ap_my_pid, SIGTERM);
@@ -832,6 +846,12 @@ static void * APR_THREAD_FUNC worker_thread(apr_thread_t *thd, void * dummy)
         last_ptrans = NULL;
 
         if (rv != APR_SUCCESS) {
+            /* We get APR_EOF during a graceful shutdown once all the connections
+             * accepted by this server process have been handled.
+             */
+            if (rv == APR_EOF) {
+                break;
+            }
             /* 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