On Feb 4, 2011, at 12:34 PM, j...@apache.org wrote: > Author: jim > Date: Fri Feb 4 20:34:47 2011 > New Revision: 1067276 > > URL: http://svn.apache.org/viewvc?rev=1067276&view=rev > Log: > Lock around the time when we're mucking w/ balancers... > > Modified: > httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c > > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1067276&r1=1067275&r2=1067276&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original) > +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Feb 4 20:34:47 > 2011 > @@ -876,8 +876,20 @@ static int balancer_handler(request_rec > params = apr_table_make(r->pool, 10); > > balancer = (proxy_balancer *)conf->balancers->elts; > - for (i = 0; i < conf->balancers->nelts; i++, balancer++) > + for (i = 0; i < conf->balancers->nelts; i++, balancer++) { > + apr_status_t rv; > + if ((rv = PROXY_GLOBAL_LOCK(balancer)) != APR_SUCCESS) { > + ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server, > + "proxy: BALANCER: (%s). Lock failed for > balancer_handler", > + balancer->name); > + } > ap_proxy_update_members(balancer, r->server, conf); > + if ((rv = PROXY_GLOBAL_UNLOCK(balancer)) != APR_SUCCESS) { > + ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server, > + "proxy: BALANCER: (%s). Unlock failed for > balancer_handler", > + balancer->name); > + } > + }
Umm, yuck. That doesn't seem right. If the lock is needed, then an update should not be done when the lock fails. And what conditions would cause us to fail an unlock? It sounds fatal. I don't see why a lock is needed here, but I haven't looked at the context enough to understand what ap_proxy_update_members() is doing (or if it could be designed lock-free). I just hate to see a loop of locking, even though I am clueless about the balancer. ....Roy
