Mailing List Archive

mpm state changes
Hi,

what do you think about adding a hook for MPMQ state changes?

AP_MPMQ_STARTING -> AP_MPMQ_RUNNING -> AP_MPMQ_STOPPING

Background: several module monitor the state to abort a loop early to not delay a child exit unnecessarily. However there seems to be no notification mechanism available (or I missed it).

While the cleanup of the child pool is a way to get notified on the exit, some resources may be worth reclaiming at the start of a (graceful) shutdown.

WDYT?

- Stefan
Re: mpm state changes [ In reply to ]
On Mon, Jun 28, 2021 at 10:28 AM Stefan Eissing
<stefan.eissing@greenbytes.de> wrote:
>
> what do you think about adding a hook for MPMQ state changes?
>
> AP_MPMQ_STARTING -> AP_MPMQ_RUNNING -> AP_MPMQ_STOPPING

Fine by me, we just need to make sure that the hooks are not called
from signal handlers (e.g. MPM prefork sets AP_MPMQ_STOPPING in the
stop_listening() handler).

Cheers;
Yann.
Re: mpm state changes [ In reply to ]
> Am 28.06.2021 um 11:18 schrieb Yann Ylavic <ylavic.dev@gmail.com>:
>
> On Mon, Jun 28, 2021 at 10:28 AM Stefan Eissing
> <stefan.eissing@greenbytes.de> wrote:
>>
>> what do you think about adding a hook for MPMQ state changes?
>>
>> AP_MPMQ_STARTING -> AP_MPMQ_RUNNING -> AP_MPMQ_STOPPING
>
> Fine by me, we just need to make sure that the hooks are not called
> from signal handlers (e.g. MPM prefork sets AP_MPMQ_STOPPING in the
> stop_listening() handler).

Right. Let's say we restrict this to changes in child processes. As in:

1. AP_DECLARE_HOOK(void,child_running,(apr_pool_t *pchild, server_rec *s))
2. AP_DECLARE_HOOK(void,child_stopping,(apr_pool_t *pchild, server_rec *s, int graceful))

1 could be called in the mpm's "child_main()" function.
2.could be called
event.c#1326: in close_listeners()
worker.c#714: listener_thread() exits its loop
winnt/child.c#1145: at start of shutdown
prefork.c: does not set _STOPPING in child processes it seems. 2 might be called in line 681
netware: is this still maintained?
os2: does not care about MPMQ_* things (is this still maintained?)
motorz: line 948
simple: does not really care about MPMQ_

- Stefan
Re: mpm state changes [ In reply to ]
On Mon, Jun 28, 2021 at 12:56 PM Stefan Eissing
<stefan.eissing@greenbytes.de> wrote:
>
> Right. Let's say we restrict this to changes in child processes. As in:
>
> 1. AP_DECLARE_HOOK(void,child_running,(apr_pool_t *pchild, server_rec *s))
> 2. AP_DECLARE_HOOK(void,child_stopping,(apr_pool_t *pchild, server_rec *s, int graceful))
>
> 1 could be called in the mpm's "child_main()" function.
+1

> 2.could be called
> event.c#1326: in close_listeners()
> worker.c#714: listener_thread() exits its loop

For event and worker I'd go with something like:
```
Index: server/mpm/event/event.c
===================================================================
--- server/mpm/event/event.c (revision 1891103)
+++ server/mpm/event/event.c (working copy)
@@ -654,6 +654,8 @@ static void signal_threads(int mode)
ap_queue_interrupt_all(worker_queue);
close_worker_sockets(); /* forcefully kill all current connections */
}
+
+ ap_run_child_stopping(pchild, mode == ST_GRACEFUL);
}

static int event_query(int query_code, int *result, apr_status_t *rv)
@@ -753,6 +755,10 @@ static void clean_child_exit(int code) __attribute
static void clean_child_exit(int code)
{
retained->mpm->mpm_state = AP_MPMQ_STOPPING;
+ if (terminate_mode == ST_INIT) {
+ ap_run_child_stopping(pchild, 0);
+ }
+
if (pchild) {
apr_pool_destroy(pchild);
}
```

> prefork.c: does not set _STOPPING in child processes it seems. 2 might be called in line 681
> motorz: line 948

For prefork and motorz maybe something like this:
```
Index: server/mpm/prefork/prefork.c
===================================================================
--- server/mpm/prefork/prefork.c (revision 1891103)
+++ server/mpm/prefork/prefork.c (working copy)
@@ -219,11 +219,14 @@ static void prefork_note_child_started(int slot, p
static void clean_child_exit(int code) __attribute__ ((noreturn));
static void clean_child_exit(int code)
{
- retained->mpm->mpm_state = AP_MPMQ_STOPPING;
-
apr_signal(SIGHUP, SIG_IGN);
apr_signal(SIGTERM, SIG_IGN);

+ retained->mpm->mpm_state = AP_MPMQ_STOPPING;
+ if (code == 0) {
+ ap_run_mpm_child_stopping(pchild, 0);
+ }
+
if (pchild) {
apr_pool_destroy(pchild);
/*
```

Yes it may be called from the just_die() signal handler, but we care
to not reenter so possibly harmless.


> winnt/child.c#1145: at start of shutdown
+1

> netware: is this still maintained?
> os2: does not care about MPMQ_* things (is this still maintained?)
> simple: does not really care about MPMQ_

No idea..


Regards;
Yann.
Re: mpm state changes [ In reply to ]
My PR with the changes supplied by Yann, for review:

https://github.com/apache/httpd/pull/199


Cheers, Stefan