On 4/12/24 12:35 PM, yla...@apache.org wrote:
> Author: ylavic
> Date: Fri Apr 12 10:35:10 2024
> New Revision: 1916925
> 
> URL: http://svn.apache.org/viewvc?rev=1916925&view=rev
> Log:
> mpm_event: Simplify pollset "good methods" vs APR_POLLSET_WAKEABLE.
> 
> * server/mpm/event/event.c(setup_threads_runtime):
>   Simplify pollset creation code.
> 
> All pollset "good methods" implement APR_POLLSET_WAKEABLE and wake-ability
> is quite important for MPM event's correctness anyway so simplify code around
> pollset creation so as not to suggest that APR_POLLSET_NODEFAULT if favored
> against APR_POLLSET_WAKEABLE.
> 
> While at it account for the wakeup pipe in the pollset_size since not all
> pollset methods seem to do it internally in APR.
> 
> 
> Modified:
>     httpd/httpd/trunk/server/mpm/event/event.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=1916925&r1=1916924&r2=1916925&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> +++ httpd/httpd/trunk/server/mpm/event/event.c Fri Apr 12 10:35:10 2024
> @@ -2630,9 +2630,9 @@ static void setup_threads_runtime(void)
>  
>      /* Create the main pollset */
>      pollset_flags = APR_POLLSET_THREADSAFE | APR_POLLSET_NOCOPY |
> -                    APR_POLLSET_NODEFAULT | APR_POLLSET_WAKEABLE;
> +                    APR_POLLSET_WAKEABLE | APR_POLLSET_NODEFAULT;
>      for (i = 0; i < sizeof(good_methods) / sizeof(good_methods[0]); i++) {
> -        rv = apr_pollset_create_ex(&event_pollset, pollset_size, pruntime,
> +        rv = apr_pollset_create_ex(&event_pollset, pollset_size + 1, 
> pruntime,

You explained this in the commit message above (thanks), but I think it would 
be good to add a comment
here in the code why +1 is added to the pollset_size to have the rational at 
hand when reading the code.

>                                     pollset_flags, good_methods[i]);
>          if (rv == APR_SUCCESS) {
>              listener_is_wakeable = 1;
> @@ -2640,19 +2640,17 @@ static void setup_threads_runtime(void)
>          }
>      }
>      if (rv != APR_SUCCESS) {
> -        pollset_flags &= ~APR_POLLSET_WAKEABLE;
> -        for (i = 0; i < sizeof(good_methods) / sizeof(good_methods[0]); i++) 
> {
> -            rv = apr_pollset_create_ex(&event_pollset, pollset_size, 
> pruntime,
> -                                       pollset_flags, good_methods[i]);
> -            if (rv == APR_SUCCESS) {
> -                break;
> -            }
> -        }
> -    }
> -    if (rv != APR_SUCCESS) {
>          pollset_flags &= ~APR_POLLSET_NODEFAULT;
> -        rv = apr_pollset_create(&event_pollset, pollset_size, pruntime,
> +        rv = apr_pollset_create(&event_pollset, pollset_size + 1, pruntime,

Same as above.

>                                  pollset_flags);
> +        if (rv == APR_SUCCESS) {
> +            listener_is_wakeable = 1;
> +        }
> +        else {
> +            pollset_flags &= ~APR_POLLSET_WAKEABLE;
> +            rv = apr_pollset_create(&event_pollset, pollset_size, pruntime,
> +                                    pollset_flags);
> +        }
>      }
>      if (rv != APR_SUCCESS) {
>          ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf, 
> APLOGNO(03103)
> 
> 
> 

Regards

RĂ¼diger

Reply via email to