mpm_event: Simplify pollset "good methods" vs APR_POLLSET_WAKEABLE.

* server/mpm/event/event.c(setup_threads_runtime):
  Simplify pollset creation code.

All pollset "good methods" implement APR_POLLSET_WAKEABLE and wake-ability
is quite important for MPM event's correctness anyway so simplify code around
pollset creation so as not to suggest that APR_POLLSET_NODEFAULT if favored
against APR_POLLSET_WAKEABLE.

While at it account for the wakeup pipe in the pollset_size since not all
pollset methods seem to do it internally in APR.

mpm_worker: Fix AH00045 about children processes not terminating timely.

* server/mpm/worker/worker.c(setup_threads_runtime):
  Create pollset with APR_POLLSET_WAKEABLE to be able to wake up the listener
  when stopping.

* server/mpm/worker/worker.c(wakeup_listener):
  Wake up the listener using the wakeup pipe (apr_pollset_wakeup).

* server/mpm/worker/worker.c(join_workers):
  Like mpm_event, don't depend on `pthread_kill(listener_thread, 0)` to check
  whether the listener has exited (this does not work on some systems), but use
  the "dying" global variable instead which is set by the listener just before
  exiting.

mpm_event,mpm_worker: Comment about pollset sizing when APR_POLLSET_WAKEABLE.

Follow up to r1916925 and r1916926.

Merges r1916925, r1916926, r1916929 from trunk
Reviewed by: rjung, ylavic, jorton

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1919134 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/changes-entries/worker_exit.txt b/changes-entries/worker_exit.txt
new file mode 100644 (file)
index 0000000..5a2e712
--- /dev/null
+++ b/changes-entries/worker_exit.txt
@@ -0,0 +1,2 @@
+  *) mpm_worker: Fix possible warning (AH00045) about children processes not
+     terminating timely.  [Yann Ylavic]

diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c
index 3672f4496344d9276c3b49a80f2559cbdf6e76a4..7e7a7e91baf6343239cd991d898138f1d3f5f4b6 100644 (file)
--- a/server/mpm/event/event.c
+++ b/server/mpm/event/event.c
@@ -2319,31 +2319,32 @@ static void setup_threads_runtime(void)
         clean_child_exit(APEXIT_CHILDFATAL);
     }
 
-    /* Create the main pollset */
+    /* Create the main pollset. When APR_POLLSET_WAKEABLE is asked we account
+     * for the wakeup pipe explicitely with pollset_size+1 because some pollset
+     * implementations don't do it implicitely in APR.
+     */
     pollset_flags = APR_POLLSET_THREADSAFE | APR_POLLSET_NOCOPY |
-                    APR_POLLSET_NODEFAULT | APR_POLLSET_WAKEABLE;
+                    APR_POLLSET_WAKEABLE | APR_POLLSET_NODEFAULT;
     for (i = 0; i < sizeof(good_methods) / sizeof(good_methods[0]); i++) {
-        rv = apr_pollset_create_ex(&event_pollset, pollset_size, pruntime,
+        rv = apr_pollset_create_ex(&event_pollset, pollset_size + 1, pruntime,
                                    pollset_flags, good_methods[i]);
         if (rv == APR_SUCCESS) {
             listener_is_wakeable = 1;
             break;
         }
     }
-    if (rv != APR_SUCCESS) {
-        pollset_flags &= ~APR_POLLSET_WAKEABLE;
-        for (i = 0; i < sizeof(good_methods) / sizeof(good_methods[0]); i++) {
-            rv = apr_pollset_create_ex(&event_pollset, pollset_size, pruntime,
-                                       pollset_flags, good_methods[i]);
-            if (rv == APR_SUCCESS) {
-                break;
-            }
-        }
-    }
     if (rv != APR_SUCCESS) {
         pollset_flags &= ~APR_POLLSET_NODEFAULT;
-        rv = apr_pollset_create(&event_pollset, pollset_size, pruntime,
+        rv = apr_pollset_create(&event_pollset, pollset_size + 1, pruntime,
                                 pollset_flags);
+        if (rv == APR_SUCCESS) {
+            listener_is_wakeable = 1;
+        }
+        else {
+            pollset_flags &= ~APR_POLLSET_WAKEABLE;
+            rv = apr_pollset_create(&event_pollset, pollset_size, pruntime,
+                                    pollset_flags);
+        }
     }
     if (rv != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf, APLOGNO(03103)

diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c
index 7b572bd14c9a8e3a146025d5eef3d1dd0ae57900..315371de121d7ad1fc318f9564e28110f58500b2 100644 (file)
--- a/server/mpm/worker/worker.c
+++ b/server/mpm/worker/worker.c
@@ -125,10 +125,11 @@ static int max_workers = 0;
 static int server_limit = 0;
 static int thread_limit = 0;
 static int had_healthy_child = 0;
-static int dying = 0;
+static volatile int dying = 0;
 static int workers_may_exit = 0;
 static int start_thread_may_exit = 0;
 static int listener_may_exit = 0;
+static int listener_is_wakeable = 0; /* Pollset supports APR_POLLSET_WAKEABLE */
 static int requests_this_child;
 static int num_listensocks = 0;
 static int resource_shortage = 0;
@@ -272,6 +273,15 @@ static void close_worker_sockets(void)
 static void wakeup_listener(void)
 {
     listener_may_exit = 1;
+
+    /* Unblock the listener if it's poll()ing */
+    if (worker_pollset && listener_is_wakeable) {
+        apr_pollset_wakeup(worker_pollset);
+    }
+
+    /* unblock the listener if it's waiting for a worker */
+    ap_queue_info_term(worker_queue_info);
+
     if (!listener_os_thread) {
         /* XXX there is an obscure path that this doesn't handle perfectly:
          *     right after listener thread is created but before
@@ -280,10 +290,6 @@ static void wakeup_listener(void)
          */
         return;
     }
-
-    /* unblock the listener if it's waiting for a worker */
-    ap_queue_info_term(worker_queue_info);
-
     /*
      * we should just be able to "kill(ap_my_pid, LISTENER_SIGNAL)" on all
      * platforms and wake up the listener thread since it is the only thread
@@ -861,6 +867,7 @@ static void create_listener_thread(thread_starter *ts)
 static void setup_threads_runtime(void)
 {
     ap_listen_rec *lr;
+    int pollset_flags;
     apr_status_t rv;
 
     /* All threads (listener, workers) and synchronization objects (queues,
@@ -893,9 +900,21 @@ static void setup_threads_runtime(void)
         clean_child_exit(APEXIT_CHILDFATAL);
     }
 
-    /* Create the main pollset */
-    rv = apr_pollset_create(&worker_pollset, num_listensocks, pruntime,
-                            APR_POLLSET_NOCOPY);
+    /* Create the main pollset. When APR_POLLSET_WAKEABLE is asked we account
+     * for the wakeup pipe explicitely with num_listensocks+1 because some
+     * pollset implementations don't do it implicitely in APR.
+     */
+    pollset_flags = APR_POLLSET_NOCOPY | APR_POLLSET_WAKEABLE;
+    rv = apr_pollset_create(&worker_pollset, num_listensocks + 1, pruntime,
+                            pollset_flags);
+    if (rv == APR_SUCCESS) {
+        listener_is_wakeable = 1;
+    }
+    else {
+        pollset_flags &= ~APR_POLLSET_WAKEABLE;
+        rv = apr_pollset_create(&worker_pollset, num_listensocks, pruntime,
+                                pollset_flags);
+    }
     if (rv != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_EMERG, rv, ap_server_conf, APLOGNO(03285)
                      "Couldn't create pollset in thread;"
@@ -1031,19 +1050,17 @@ static void join_workers(apr_thread_t *listener, apr_thread_t **threads,
          */
 
         iter = 0;
-        while (iter < 10 &&
-#ifdef HAVE_PTHREAD_KILL
-               pthread_kill(*listener_os_thread, 0)
-#else
-               kill(ap_my_pid, 0)
-#endif
-               == 0) {
-            /* listener not dead yet */
-            apr_sleep(apr_time_make(0, 500000));
+        while (!dying) {
+            apr_sleep(apr_time_from_msec(500));
+            if (dying || ++iter > 10) {
+                break;
+            }
+            /* listener has not stopped accepting yet */
+            ap_log_error(APLOG_MARK, APLOG_TRACE1, 0, ap_server_conf,
+                         "listener has not stopped accepting yet (%d iter)", iter);
             wakeup_listener();
-            ++iter;
         }
-        if (iter >= 10) {
+        if (iter > 10) {
             ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, ap_server_conf, APLOGNO(00276)
                          "the listener thread didn't exit");
         }