Mailing List Archive

On 4/12/24 12:35 PM, ylavic@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