httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
Subject Re: svn commit: r1783755 - /httpd/httpd/trunk/server/mpm/event/event.c
Date Mon, 20 Feb 2017 14:16:51 GMT
The below got me thinking...

Right now, the pool allocator mutex is only used when, well,
allocator_alloc() is called, which means that sometimes
apr_palloc(), for example, can be thread-safeish and sometimes
not, depending on whether or not the active node has enough
space.

For 1.6 and later, it might be nice to actually protect the
adjustment of the active node, et.al. to, if a mutex is present,
always be thread-safe... that is, always when we "alloc" memory,
even when/if we do/don't called allocator_alloc().

Thoughts?

> On Feb 20, 2017, at 8:38 AM, ylavic@apache.org wrote:
> 
> Author: ylavic
> Date: Mon Feb 20 13:38:03 2017
> New Revision: 1783755
> 
> URL: http://svn.apache.org/viewvc?rev=1783755&view=rev
> Log:
> mpm_event: use a mutex for ptrans' allocator to be safe with concurrent
> creation and destruction of its subpools, like with mod_http2.
> 
> 
> 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=1783755&r1=1783754&r2=1783755&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> +++ httpd/httpd/trunk/server/mpm/event/event.c Mon Feb 20 13:38:03 2017
> @@ -2096,6 +2096,7 @@ static void * APR_THREAD_FUNC listener_t
>                 }
>                 if (!listeners_disabled) {
>                     void *csd = NULL;
> +                    apr_thread_mutex_t *mutex;
>                     ap_listen_rec *lr = (ap_listen_rec *) pt->baton;
>                     apr_pool_t *ptrans;         /* Pool for per-transaction stuff */
>                     ap_pop_pool(&ptrans, worker_queue_info);
> @@ -2105,20 +2106,44 @@ static void * APR_THREAD_FUNC listener_t
>                         apr_allocator_t *allocator;
> 
>                         apr_allocator_create(&allocator);
> -                        apr_allocator_max_free_set(allocator,
> -                                                   ap_max_mem_free);
> +                        apr_allocator_max_free_set(allocator, ap_max_mem_free);
>                         apr_pool_create_ex(&ptrans, pconf, NULL, allocator);
> -                        apr_allocator_owner_set(allocator, ptrans);
>                         if (ptrans == NULL) {
>                             ap_log_error(APLOG_MARK, APLOG_CRIT, rc,
>                                          ap_server_conf, APLOGNO(03097)
>                                          "Failed to create transaction pool");
> +                            apr_allocator_destroy(allocator);
>                             signal_threads(ST_GRACEFUL);
>                             return NULL;
>                         }
> +                        apr_allocator_owner_set(allocator, ptrans);
>                     }
>                     apr_pool_tag(ptrans, "transaction");
> 
> +                    /* We need a mutex in the allocator to synchronize ptrans'
> +                     * children creations/destructions, but this mutex ought to
> +                     * live in ptrans itself to avoid leaks, hence it's cleared
> +                     * in ap_push_pool(). We could recycle some pconf's mutexes
> +                     * like we do for ptrans subpools, but that'd need another
> +                     * synchronization mechanism, whereas creating a pthread
> +                     * mutex (unix here!) is really as simple/fast as a static
> +                     * PTHREAD_MUTEX_INIT assignment, so let's not bother and
> +                     * create the mutex for each ptrans (recycled or not).
> +                     */
> +                    rc = apr_thread_mutex_create(&mutex,
> +                                                 APR_THREAD_MUTEX_DEFAULT,
> +                                                 ptrans);
> +                    if (rc != APR_SUCCESS) {
> +                        ap_log_error(APLOG_MARK, APLOG_CRIT, rc,
> +                                     ap_server_conf, APLOGNO()
> +                                     "Failed to create transaction pool mutex");
> +                        ap_push_pool(worker_queue_info, ptrans);
> +                        signal_threads(ST_GRACEFUL);
> +                        return NULL;
> +                    }
> +                    apr_allocator_mutex_set(apr_pool_allocator_get(ptrans),
> +                                            mutex);
> +
>                     get_worker(&have_idle_worker, 1, &workers_were_busy);
>                     rc = lr->accept_func(&csd, lr, ptrans);
> 
> 
> 


Mime
View raw message