On Thu, Sep 26, 2019 at 8:20 AM Pluem, Ruediger, Vodafone Group
wrote:
>
> > -Ursprüngliche Nachricht-
> > Von: Yann Ylavic
> >
> > Likewise, I think the MPMs themselves shouldn't use pchild for their
> > internal allocations possibly still in use at exit().
> > So v2 (attached) may be the thing..
>
> Hm, haven't checked, but aren't there any cleanups that should run and
> currently run before exit that will not run any longer when we tie
> stuff to pconf instead of pchild?
> I guess pure allocations are not a problem, since the process dies,
> but I would be a little worried about other OS resources like
> shared memory or locks not being cleaned up properly.
I think you are right, proc mutexes at least need to cleanup properly
on child exit.
I updated the patch (attached) to keep them on pchild.
> Regarding the watchdog threads I guess we could handle this
> like Stefan suggested by handling it similar to still running connections.
> Give them a grace period and kill them afterwards during regular shutdown.
> For an immediate shutdown kill them off directly.
Killing threads is going to be hard to achieve, all the more so in a
portable way. There is no apr_thread_kill() for instance,
pthread_kill() is not suitable, I know of tgkill() on linux...
But we shouldn't take that road IMHO, and regarding the state of
shared/proc resources potentially used by these threads it looks like
a can of worms..
Asking for watchdog callbacks (including third-parties') to
[un]gracefully stop is not something in the current "contract"
unfortunately, we are quite weaponless here I'm afraid.
So I can only think of _exit() like in attached v3, although in
addition to not run atexit() handlers _exit() also potentially does
not flush stdios, but all fds are closed so pending outputs should
still finish (for whatever that means in linux/BSD docs..).
This is still going to be racy with anything initialized on pchild
though, like mod_ssl caches mutexes (session, stapling) :/
Regards,
Yann.
Index: modules/core/mod_watchdog.c
===
--- modules/core/mod_watchdog.c (revision 1866998)
+++ modules/core/mod_watchdog.c (working copy)
@@ -567,8 +567,8 @@ static void wd_child_init_hook(apr_pool_t *p, serv
"Watchdog: nothing configured?");
return;
}
-if ((wl = ap_list_provider_names(p, AP_WATCHDOG_PGROUP,
-AP_WATCHDOG_CVERSION))) {
+if ((wl = ap_list_provider_names(wd_server_conf->pool, AP_WATCHDOG_PGROUP,
+ AP_WATCHDOG_CVERSION))) {
const ap_list_provider_names_t *wn;
int i;
wn = (ap_list_provider_names_t *)wl->elts;
Index: server/mpm/event/event.c
===
--- server/mpm/event/event.c (revision 1866998)
+++ server/mpm/event/event.c (working copy)
@@ -772,7 +772,7 @@ static void clean_child_exit(int code)
event_note_child_killed(/* slot */ 0, 0, 0);
}
-exit(code);
+_exit(code);
}
static void just_die(int sig)
@@ -2799,9 +2799,9 @@ static void child_main(int child_num_arg, int chil
* and we want a 0 entry to indicate a thread which was not created
*/
threads = ap_calloc(threads_per_child, sizeof(apr_thread_t *));
-ts = apr_palloc(pchild, sizeof(*ts));
+ts = apr_palloc(pconf, sizeof(*ts));
-apr_threadattr_create(_attr, pchild);
+apr_threadattr_create(_attr, pconf);
/* 0 means PTHREAD_CREATE_JOINABLE */
apr_threadattr_detach_set(thread_attr, 0);
@@ -2821,7 +2821,7 @@ static void child_main(int child_num_arg, int chil
ts->threadattr = thread_attr;
rv = apr_thread_create(_thread_id, thread_attr, start_threads,
- ts, pchild);
+ ts, pconf);
if (rv != APR_SUCCESS) {
ap_log_error(APLOG_MARK, APLOG_ALERT, rv, ap_server_conf, APLOGNO(00480)
"apr_thread_create: unable to create worker thread");
Index: server/mpm/motorz/motorz.c
===
--- server/mpm/motorz/motorz.c (revision 1866998)
+++ server/mpm/motorz/motorz.c (working copy)
@@ -598,7 +598,7 @@ static void clean_child_exit(int code)
}
ap_mpm_pod_close(my_bucket->pod);
-exit(code);
+_exit(code);
}
#if 0 /* unused for now */
@@ -812,7 +812,7 @@ static void child_main(motorz_core_t *mz, int chil
ap_run_child_init(pchild, ap_server_conf);
-ap_create_sb_handle(, pchild, my_child_num, 0);
+ap_create_sb_handle(, mz->pool, my_child_num, 0);
ap_update_child_status(sbh, SERVER_READY, NULL);
Index: server/mpm/prefork/prefork.c
===
--- server/mpm/prefork/prefork.c (revision 1866998)
+++ server/mpm/prefork/prefork.c (working copy)
@@ -234,7 +234,7 @@ static void clean_child_exit(int code)