On Sun, 7 Jul 2013, j...@apache.org wrote: > Author: jim > Date: Sun Jul 7 14:05:37 2013 > New Revision: 1500437 > > URL: http://svn.apache.org/r1500437 > Log: > conf->mutex is not used... Also, ensure that pool > use is protected > > Modified: > httpd/httpd/trunk/modules/proxy/proxy_util.c > > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1500437&r1=1500436&r2=1500437&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Sun Jul 7 14:05:37 2013 > @@ -2900,10 +2900,10 @@ PROXY_DECLARE(apr_status_t) ap_proxy_syn > } > if (!found) { > proxy_worker **runtime; > + apr_global_mutex_lock(proxy_mutex); > runtime = apr_array_push(b->workers); > - apr_global_mutex_lock(conf->mutex); > *runtime = apr_palloc(conf->pool, sizeof(proxy_worker)); > - apr_global_mutex_unlock(conf->mutex); > + apr_global_mutex_unlock(proxy_mutex); > (*runtime)->hash = shm->hash; > (*runtime)->context = NULL; > (*runtime)->cp = NULL;
The extension of b->workers was not what I meant. I mean the call of ap_proxy_initialize_worker with conf->pool a few lines later, missing that ap_proxy_initialize_worker did the locking itself. But your change to protect b->workers may actually highlight another problem: b is a proxy_balancer which seems to be created by ap_proxy_define_balancer(). ap_proxy_define_balancer is however always called with cmd->pool as argument, which is usually pconf. But use of pconf is unsafe after the threads have been started. Am I correct? But I would consider that a different issue. Therefore I have voted +1 to the proposed fixes which are definitely some improvement. Reading the mod_proxy code, I remembered the feature request for thread-safe apr pools with internal mutexes and automatic locking. Do you think such thread-safe pools would help for the proxy balancer pool handling without sacrificing too much performance? Of course the connection and request pools would stay normal, non-thread-safe pools. Cheers, Stefan
