You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2018/01/18 14:19:28 UTC

svn commit: r1821499 - in /httpd/httpd/trunk: CHANGES server/mpm/event/event.c server/mpm/worker/worker.c

Author: ylavic
Date: Thu Jan 18 14:19:28 2018
New Revision: 1821499

URL: http://svn.apache.org/viewvc?rev=1821499&view=rev
Log:
mpm_event,worker: Mask signals for threads created by modules in child init.

PR 62009, so that they don't receive (implicitely) the ones meant for the MPM.

Inspired by: Armin Abfalterer <a.abfalterer gmail.com>
Proposed by: Yann Ylavic


Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/server/mpm/event/event.c
    httpd/httpd/trunk/server/mpm/worker/worker.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1821499&r1=1821498&r2=1821499&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Thu Jan 18 14:19:28 2018
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.1
 
+  *) mpm_event,worker: Mask signals for threads created by modules in child
+     init, so that they don't receive (implicitely) the ones meant for the MPM.
+     PR 62009. [Armin Abfalterer <a.abfalterer gmail.com>, Yann Ylavic]
+
   *) mpm_event: Update scoreboard status for KeepAlive state.  [Yann Ylavic]
 
   *) core, mpm_event: Avoid a small memory leak of the scoreboard handle, for

Modified: httpd/httpd/trunk/server/mpm/event/event.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1821499&r1=1821498&r2=1821499&view=diff
==============================================================================
--- httpd/httpd/trunk/server/mpm/event/event.c (original)
+++ httpd/httpd/trunk/server/mpm/event/event.c Thu Jan 18 14:19:28 2018
@@ -2663,19 +2663,11 @@ static void child_main(int child_num_arg
     /*stuff to do before we switch id's, so we have permissions. */
     ap_reopen_scoreboard(pchild, NULL, 0);
 
+    /* done with init critical section */
     if (ap_run_drop_privileges(pchild, ap_server_conf)) {
         clean_child_exit(APEXIT_CHILDFATAL);
     }
 
-    apr_thread_mutex_create(&g_timer_skiplist_mtx, APR_THREAD_MUTEX_DEFAULT, pchild);
-    APR_RING_INIT(&timer_free_ring, timer_event_t, link);
-    apr_pool_create(&pskip, pchild);
-    apr_skiplist_init(&timer_skiplist, pskip);
-    apr_skiplist_set_compare(timer_skiplist, timer_comp, timer_comp);
-    ap_run_child_init(pchild, ap_server_conf);
-
-    /* done with init critical section */
-
     /* Just use the standard apr_setup_signal_thread to block all signals
      * from being received.  The child processes no longer use signals for
      * any communication with the parent process.
@@ -2687,6 +2679,14 @@ static void child_main(int child_num_arg
         clean_child_exit(APEXIT_CHILDFATAL);
     }
 
+    ap_run_child_init(pchild, ap_server_conf);
+
+    apr_thread_mutex_create(&g_timer_skiplist_mtx, APR_THREAD_MUTEX_DEFAULT, pchild);
+    APR_RING_INIT(&timer_free_ring, timer_event_t, link);
+    apr_pool_create(&pskip, pchild);
+    apr_skiplist_init(&timer_skiplist, pskip);
+    apr_skiplist_set_compare(timer_skiplist, timer_comp, timer_comp);
+
     if (ap_max_requests_per_child) {
         conns_this_child = ap_max_requests_per_child;
     }

Modified: httpd/httpd/trunk/server/mpm/worker/worker.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/worker/worker.c?rev=1821499&r1=1821498&r2=1821499&view=diff
==============================================================================
--- httpd/httpd/trunk/server/mpm/worker/worker.c (original)
+++ httpd/httpd/trunk/server/mpm/worker/worker.c Thu Jan 18 14:19:28 2018
@@ -1112,17 +1112,16 @@ static void child_main(int child_num_arg
         clean_child_exit(APEXIT_CHILDFATAL);
     }
 
+    /* done with init critical section */
     if (ap_run_drop_privileges(pchild, ap_server_conf)) {
         clean_child_exit(APEXIT_CHILDFATAL);
     }
 
-    ap_run_child_init(pchild, ap_server_conf);
-
-    /* done with init critical section */
-
     /* Just use the standard apr_setup_signal_thread to block all signals
      * from being received.  The child processes no longer use signals for
-     * any communication with the parent process.
+     * any communication with the parent process. Let's also do this before
+     * child_init() hooks are called and possibly create threads that
+     * otherwise could "steal" (implicitely) MPM's signals.
      */
     rv = apr_setup_signal_thread();
     if (rv != APR_SUCCESS) {
@@ -1131,6 +1130,8 @@ static void child_main(int child_num_arg
         clean_child_exit(APEXIT_CHILDFATAL);
     }
 
+    ap_run_child_init(pchild, ap_server_conf);
+
     if (ap_max_requests_per_child) {
         requests_this_child = ap_max_requests_per_child;
     }



Re: svn commit: r1821499 - in /httpd/httpd/trunk: CHANGES server/mpm/event/event.c server/mpm/worker/worker.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Jan 18, 2018 at 3:43 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
> Is there a reason why ap_run_child_init and the skiplist stuff are now in reverse order?

Nope, I realized that afterward, and actually if some child_init()
plays with event's timers that will crash.
So I reverted and re-commited without this change (which also helps
backport btw).

Thanks for the review,
Yann.

Re: svn commit: r1821499 - in /httpd/httpd/trunk: CHANGES server/mpm/event/event.c server/mpm/worker/worker.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 01/18/2018 03:19 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Thu Jan 18 14:19:28 2018
> New Revision: 1821499
> 
> URL: http://svn.apache.org/viewvc?rev=1821499&view=rev
> Log:
> mpm_event,worker: Mask signals for threads created by modules in child init.
> 
> PR 62009, so that they don't receive (implicitely) the ones meant for the MPM.
> 
> Inspired by: Armin Abfalterer <a.abfalterer gmail.com>
> Proposed by: Yann Ylavic
> 
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/server/mpm/event/event.c
>     httpd/httpd/trunk/server/mpm/worker/worker.c
> 
>
> Modified: httpd/httpd/trunk/server/mpm/event/event.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1821499&r1=1821498&r2=1821499&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> +++ httpd/httpd/trunk/server/mpm/event/event.c Thu Jan 18 14:19:28 2018
> @@ -2663,19 +2663,11 @@ static void child_main(int child_num_arg
>      /*stuff to do before we switch id's, so we have permissions. */
>      ap_reopen_scoreboard(pchild, NULL, 0);
>  
> +    /* done with init critical section */
>      if (ap_run_drop_privileges(pchild, ap_server_conf)) {
>          clean_child_exit(APEXIT_CHILDFATAL);
>      }
>  
> -    apr_thread_mutex_create(&g_timer_skiplist_mtx, APR_THREAD_MUTEX_DEFAULT, pchild);
> -    APR_RING_INIT(&timer_free_ring, timer_event_t, link);
> -    apr_pool_create(&pskip, pchild);
> -    apr_skiplist_init(&timer_skiplist, pskip);
> -    apr_skiplist_set_compare(timer_skiplist, timer_comp, timer_comp);
> -    ap_run_child_init(pchild, ap_server_conf);
> -
> -    /* done with init critical section */
> -
>      /* Just use the standard apr_setup_signal_thread to block all signals
>       * from being received.  The child processes no longer use signals for
>       * any communication with the parent process.
> @@ -2687,6 +2679,14 @@ static void child_main(int child_num_arg
>          clean_child_exit(APEXIT_CHILDFATAL);
>      }
>  
> +    ap_run_child_init(pchild, ap_server_conf);
> +
> +    apr_thread_mutex_create(&g_timer_skiplist_mtx, APR_THREAD_MUTEX_DEFAULT, pchild);
> +    APR_RING_INIT(&timer_free_ring, timer_event_t, link);
> +    apr_pool_create(&pskip, pchild);
> +    apr_skiplist_init(&timer_skiplist, pskip);
> +    apr_skiplist_set_compare(timer_skiplist, timer_comp, timer_comp);

Is there a reason why ap_run_child_init and the skiplist stuff are now in reverse order?

> +
>      if (ap_max_requests_per_child) {
>          conns_this_child = ap_max_requests_per_child;
>      }
> 


Regards

RĂ¼diger