You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Stefan Eissing <st...@greenbytes.de> on 2021/06/28 08:28:39 UTC

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

Posted by Stefan Eissing <st...@greenbytes.de>.
My PR with the changes supplied by Yann, for review:

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


Cheers, Stefan

Re: mpm state changes

Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Jun 28, 2021 at 12:56 PM Stefan Eissing
<st...@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

Posted by Stefan Eissing <st...@greenbytes.de>.

> Am 28.06.2021 um 11:18 schrieb Yann Ylavic <yl...@gmail.com>:
> 
> On Mon, Jun 28, 2021 at 10:28 AM Stefan Eissing
> <st...@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

Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Jun 28, 2021 at 10:28 AM Stefan Eissing
<st...@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.