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
