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.

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

diff --git a/os/unix/unixd.c b/os/unix/unixd.c
index 07a9bef754c5102e655aec37eb48d613fe637105..b17b6ac2e3dce0a9b606f4386d3f4f3fb755b748 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
@@ -494,6 +510,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 e67c001d3718dc099bb46b1fd82f86e0f21d5603..d0f6b3f808f63d52457a6b0b5e594d609d119b4d 100644 (file)
--- a/server/main.c
+++ b/server/main.c
@@ -295,13 +295,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;
@@ -346,8 +363,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]);
@@ -504,10 +522,6 @@ int main(int argc, const char * const argv[])
     }
 #endif
 
-    /* Deregister all hooks (lastly) when done with pconf */
-    apr_pool_cleanup_register(pconf, NULL, deregister_all_hooks,
-                              apr_pool_cleanup_null);
-
     apr_pool_create(&pcommands, ap_pglobal);
     apr_pool_tag(pcommands, "pcommands");
     ap_server_pre_read_config  = apr_array_make(pcommands, 1,
@@ -754,12 +768,7 @@ int main(int argc, const char * const argv[])
 
     do {
         ap_main_state = AP_SQ_MS_DESTROY_CONFIG;
-        apr_pool_clear(pconf);
-        ap_clear_auth_internal();
-
-        /* Deregister all hooks (lastly) when done with pconf */
-        apr_pool_cleanup_register(pconf, NULL, deregister_all_hooks,
-                                  apr_pool_cleanup_null);
+        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 d61c85e6a71cc4a317cfcdbee0c2611480766d39..0bf2dbe7755236be660be7c5bd20e0e71a8b882f 100644 (file)
--- a/server/mpm/event/event.c
+++ b/server/mpm/event/event.c
@@ -3149,9 +3149,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/motorz/motorz.c b/server/mpm/motorz/motorz.c
index b0ffc4d1cb5e1d414ac861e1a6920eb58c7629d8..d8d95222952df33ad952bfa6448bd58b632e44bc 100644 (file)
--- a/server/mpm/motorz/motorz.c
+++ b/server/mpm/motorz/motorz.c
@@ -1104,9 +1104,6 @@ static int motorz_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
         ap_scoreboard_image->global->running_generation = mz->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/prefork/prefork.c b/server/mpm/prefork/prefork.c
index 6649efe7e73cfb0572f60f2b21cc7454d78c7ea1..f9b9933bd32cd6385d65b9eed5209f3e7caf414c 100644 (file)
--- a/server/mpm/prefork/prefork.c
+++ b/server/mpm/prefork/prefork.c
@@ -864,9 +864,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 969a8196bd048b21f25c4b3f912f8d280ffaed4a..3b93f3ceac59d227893629c6c68fbd44911499cd 100644 (file)
--- a/server/mpm/worker/worker.c
+++ b/server/mpm/worker/worker.c
@@ -1672,9 +1672,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 fb589fda2f71a5299dc6dfaa58fcf2a7d316e502..29bc3a2b00b0da3f46433ae1481c5020121b199b 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;
 }