httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: worker MPM on trunk does not shut down cleanly
Date Sun, 03 Aug 2008 18:44:57 GMT


On 08/03/2008 06:28 PM, Jim Jagielski wrote:
> 
> On Aug 1, 2008, at 4:44 AM, Mladen Turk wrote:
> 
>> Ruediger Pluem wrote:
>>> Ok, this is caused by http://svn.apache.org/viewvc?rev=677505&view=rev
>>> This is the reslist pre_cleanup patch. I don't know why so far, but as
>>> I have a proxy configuration I suspect that it blocks on tearing down
>>> the proxy connection pools.
>>
>> Here is the fix for trunk.
>>
>> Index: proxy_util.c
>> ===================================================================
>> --- proxy_util.c        (revision 681621)
>> +++ proxy_util.c        (working copy)
>> @@ -1939,10 +1939,11 @@
>>                                 worker->hmax, worker->ttl,
>>                                 connection_constructor, 
>> connection_destructor,
>>                                 worker, worker->cp->pool);
>> -
>> +#if 0
>>         apr_pool_cleanup_register(worker->cp->pool, (void *)worker,
>>                                   conn_pool_cleanup,
>>                                   apr_pool_cleanup_null);
>> +#endif
>>
>>
>>
>> Note that because of using pre_cleanup in reslist we don't need
>> the extra registered cleanup (conn_pool_cleanup),
>> just to make sure the ordering is correct.
>> This was bogus anyhow, because we were destroying the reslist in
>> cleanup (that already has it's own cleanup), so the ordering of
>> cleanup callbacks was essential.
>>
> 
> I wonder how many other just uses in other modules would be just
> so affected?
> 
> So does this mean that trunk is now based on a "broken" or
> incompatible version of apr? Do we need to now break off
> trunk to 2.4 and baseline APR 1.3 to allow trunk to now work
> with an incompatible APR rev?

As far as this specific issue is concerned, IMHO no. The following
patch fixes the behaviour on trunk (with apr-util trunk) and does no
harm on 2.2.x:


Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c  (Revision 681204)
+++ modules/proxy/proxy_util.c  (Arbeitskopie)
@@ -1380,7 +1380,6 @@
      proxy_worker *worker = (proxy_worker *)theworker;
      if (worker->cp->res) {
          worker->cp->pool = NULL;
-        apr_reslist_destroy(worker->cp->res);
      }
      return APR_SUCCESS;
  }

Why?
Trunk (with apr-util trunk):

At the point of time we would execute apr_reslist_destroy the reslist is already destroyed,
because we are in a cleanup of the same pool where the reslist registered itself as
precleanup. This causes the lock at shutdown.

2.2.x:
Calling apr_reslist_destroy is not really useful and needed in this case as we are in a cleanup
that was registered against the same pool that is used by the reslist. As it was registered
*after* the reslist was created it just runs *before* the reslist cleanup would run. This
is somewhat pointless here and we could leave the job of destroying the reslist to the
reslist cleanup.

Nevertheless I think that the precleanup code in apr trunk and the changes to the reslist
in apr-util trunk are not backportable just because of the example above: Code may break
if you change an apr / apr-util 1.3 release under the hood. This should not happen.

Regards

RĂ¼diger





Mime
View raw message