Merge r1809881, r1809973, r1809976, r1812075 from trunk:

core: deregister all hooks before leaving pconf, otherwise some late cleanup
or function call (e.g. ap_log) may use one while DSOs are unloaded.

See PR 61558 (double/second fault).

core, MPMs unix: follow up to r1809881.

Deregister all hooks first (in pre_cleanup), by doing it last we could still
have had them run when DSOs were unloaded.

Likewise, avoid double faults when handling fatal signals by restoring the
default handler before pconf is cleared (we can't ap_log_error there).

Finally, we need to ignore sig_term/restart (do nothing) when the main
process is exiting (i.e. ap_pglobal is destroyed), since retained_data are
freed.

Aimed to fix all faults in PR 61558.

MPMs unix: follow up to r1809881 and r1809973.

unset_signals() is called when ap_pglobal is destroyed too.

Follow up to r1809881: CHANGES entry.

Submitted by: ylavic
Reviewed by: ylavic, jim, covener

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

diff --git a/CHANGES b/CHANGES
index d99d07bb99350afb90453c54ebe74e046f1bac85..1d528ea7eb382cc3edccc6efdf84f3b3d34352fa 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.30
  
+  *) core: A signal received while stopping could have crashed the main
+     process.  PR 61558.  [Yann Ylavic]
+
   *) mod_ssl: support for mod_md added. [Stefan Eissing]
   
   *) mod_proxy_html: process parsed comments immediately. 

diff --git a/os/unix/unixd.c b/os/unix/unixd.c
index 07a9bef754c5102e655aec37eb48d613fe637105..7f71d1a6425df563d71de2ca27aa420577f450ff 100644 (file)
--- a/os/unix/unixd.c
+++ b/os/unix/unixd.c
@@ -437,11 +437,19 @@ AP_DECLARE(apr_status_t) ap_unixd_accept(void **accepted, ap_listen_rec *lr,
 /* Unixes MPMs' */
 
 static ap_unixd_mpm_retained_data *retained_data = NULL;
+static apr_status_t retained_data_cleanup(void *unused)
+{
+    (void)unused;
+    retained_data = NULL;
+    return APR_SUCCESS;
+}
+
 AP_DECLARE(ap_unixd_mpm_retained_data *) ap_unixd_mpm_get_retained_data()
 {
     if (!retained_data) {
         retained_data = ap_retained_data_create("ap_unixd_mpm_retained_data",
                                                 sizeof(*retained_data));
+        apr_pool_pre_cleanup_register(ap_pglobal, NULL, retained_data_cleanup);
         retained_data->mpm_state = AP_MPMQ_STARTING;
     }
     return retained_data;
@@ -449,6 +457,10 @@ AP_DECLARE(ap_unixd_mpm_retained_data *) ap_unixd_mpm_get_retained_data()
 
 static void sig_term(int sig)
 {
+    if (!retained_data) {
+        /* Main process (ap_pglobal) is dying */
+        return;
+    }
     retained_data->mpm_state = AP_MPMQ_STOPPING;
     if (retained_data->shutdown_pending
             && (retained_data->is_ungraceful
@@ -465,6 +477,10 @@ static void sig_term(int sig)
 
 static void sig_restart(int sig)
 {
+    if (!retained_data) {
+        /* Main process (ap_pglobal) is dying */
+        return;
+    }
     retained_data->mpm_state = AP_MPMQ_STOPPING;
     if (retained_data->restart_pending
             && (retained_data->is_ungraceful
@@ -481,6 +497,10 @@ static void sig_restart(int sig)
 
 static apr_status_t unset_signals(void *unused)
 {
+    if (!retained_data) {
+        /* Main process (ap_pglobal) is dying */
+        return APR_SUCCESS;
+    }
     retained_data->shutdown_pending = retained_data->restart_pending = 0;
     retained_data->was_graceful = !retained_data->is_ungraceful;
     retained_data->is_ungraceful = 0;
@@ -494,6 +514,10 @@ AP_DECLARE(void) ap_unixd_mpm_set_signals(apr_pool_t *pconf, int one_process)
     struct sigaction sa;
 #endif
 
+    if (!one_process) {
+        ap_fatal_signal_setup(ap_server_conf, pconf);
+    }
+
     /* Signals' handlers depend on retained data */
     (void)ap_unixd_mpm_get_retained_data();
 

diff --git a/server/main.c b/server/main.c
index ec0ea68d9b940979cf6ebf80ebb80311335f638a..3a4b1a6254fe5bd06f3baec076e901951e4b09cb 100644 (file)
--- a/server/main.c
+++ b/server/main.c
@@ -273,6 +273,30 @@ static int abort_on_oom(int retcode)
     return retcode; /* unreachable, hopefully. */
 }
 
+/* Deregister all hooks when clearing pconf (pre_cleanup).
+ * TODO: have a hook to deregister and run them from here?
+ *       ap_clear_auth_internal() is already a candidate.
+ */
+static apr_status_t deregister_all_hooks(void *unused)
+{
+    (void)unused;
+    ap_clear_auth_internal();
+    apr_hook_deregister_all();
+    return APR_SUCCESS;
+}
+
+static void reset_process_pconf(process_rec *process)
+{
+    if (process->pconf) {
+        apr_pool_clear(process->pconf);
+    }
+    else {
+        apr_pool_create(&process->pconf, process->pool);
+        apr_pool_tag(process->pconf, "pconf");
+    }
+    apr_pool_pre_cleanup_register(process->pconf, NULL, deregister_all_hooks);
+}
+
 static process_rec *init_process(int *argc, const char * const * *argv)
 {
     process_rec *process;
@@ -317,8 +341,9 @@ static process_rec *init_process(int *argc, const char * const * *argv)
     process = apr_palloc(cntx, sizeof(process_rec));
     process->pool = cntx;
 
-    apr_pool_create(&process->pconf, process->pool);
-    apr_pool_tag(process->pconf, "pconf");
+    process->pconf = NULL;
+    reset_process_pconf(process);
+
     process->argc = *argc;
     process->argv = *argv;
     process->short_name = apr_filepath_name_get((*argv)[0]);
@@ -721,9 +746,7 @@ int main(int argc, const char * const argv[])
 
     do {
         ap_main_state = AP_SQ_MS_DESTROY_CONFIG;
-        apr_hook_deregister_all();
-        apr_pool_clear(pconf);
-        ap_clear_auth_internal();
+        reset_process_pconf(process);
 
         ap_main_state = AP_SQ_MS_CREATE_CONFIG;
         ap_config_generation++;

diff --git a/server/mpm/event/event.c b/server/mpm/event/event.c
index bd3ad3c9989d10ee18f7640ea914a27c46b87030..2efde3982dee377a589a66edf10163802e1d50ff 100644 (file)
--- a/server/mpm/event/event.c
+++ b/server/mpm/event/event.c
@@ -2878,9 +2878,6 @@ static int event_run(apr_pool_t * _pconf, apr_pool_t * plog, server_rec * s)
         ap_scoreboard_image->global->running_generation = retained->mpm->my_generation;
     }
 
-    if (!one_process) {
-        ap_fatal_signal_setup(ap_server_conf, pconf);
-    }
     ap_unixd_mpm_set_signals(pconf, one_process);
 
     /* Don't thrash since num_buckets depends on the

diff --git a/server/mpm/prefork/prefork.c b/server/mpm/prefork/prefork.c
index a386a757ffd2d11ed05f3d100dff90587e966d56..3fb328467d9aef2a9e4f19e74d98498285deb38b 100644 (file)
--- a/server/mpm/prefork/prefork.c
+++ b/server/mpm/prefork/prefork.c
@@ -863,9 +863,6 @@ static int prefork_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
         ap_scoreboard_image->global->running_generation = retained->mpm->my_generation;
     }
 
-    if (!one_process) {
-        ap_fatal_signal_setup(ap_server_conf, pconf);
-    }
     ap_unixd_mpm_set_signals(pconf, one_process);
 
     if (one_process) {

diff --git a/server/mpm/worker/worker.c b/server/mpm/worker/worker.c
index d2147bf5d9406270332e70db948cd2cf4c08e64b..776ae7fc2de40a4015c56979b5fb84df2c68ad6b 100644 (file)
--- a/server/mpm/worker/worker.c
+++ b/server/mpm/worker/worker.c
@@ -1671,9 +1671,6 @@ static int worker_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
         ap_scoreboard_image->global->running_generation = retained->mpm->my_generation;
     }
 
-    if (!one_process) {
-        ap_fatal_signal_setup(ap_server_conf, pconf);
-    }
     ap_unixd_mpm_set_signals(pconf, one_process);
 
     /* Don't thrash since num_buckets depends on the

diff --git a/server/mpm_unix.c b/server/mpm_unix.c
index 2f3d20e4dfe793a1ff31af1779adb0fd04b79e4f..1800f5dabf7cad5e8af2c5bb19146c3e576fe43e 100644 (file)
--- a/server/mpm_unix.c
+++ b/server/mpm_unix.c
@@ -1009,6 +1009,33 @@ AP_DECLARE(apr_status_t) ap_fatal_signal_child_setup(server_rec *s)
     return APR_SUCCESS;
 }
 
+/* We can't call sig_coredump (ap_log_error) once pconf is destroyed, so
+ * avoid double faults by restoring each default signal handler on cleanup.
+ */
+static apr_status_t fatal_signal_cleanup(void *unused)
+{
+    (void)unused;
+
+    apr_signal(SIGSEGV, SIG_DFL);
+#ifdef SIGBUS
+    apr_signal(SIGBUS, SIG_DFL);
+#endif /* SIGBUS */
+#ifdef SIGABORT
+    apr_signal(SIGABORT, SIG_DFL);
+#endif /* SIGABORT */
+#ifdef SIGABRT
+    apr_signal(SIGABRT, SIG_DFL);
+#endif /* SIGABRT */
+#ifdef SIGILL
+    apr_signal(SIGILL, SIG_DFL);
+#endif /* SIGILL */
+#ifdef SIGFPE
+    apr_signal(SIGFPE, SIG_DFL);
+#endif /* SIGFPE */
+
+    return APR_SUCCESS;
+}
+
 AP_DECLARE(apr_status_t) ap_fatal_signal_setup(server_rec *s,
                                                apr_pool_t *in_pconf)
 {
@@ -1071,6 +1098,8 @@ AP_DECLARE(apr_status_t) ap_fatal_signal_setup(server_rec *s,
 
     pconf = in_pconf;
     parent_pid = my_pid = getpid();
+    apr_pool_cleanup_register(pconf, NULL, fatal_signal_cleanup,
+                              fatal_signal_cleanup);
 
     return APR_SUCCESS;
 }