apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Darroch <chr...@pearsoncmg.com>
Subject Re: Changing the order of cleanup for some core objects
Date Mon, 21 Jul 2008 17:11:44 GMT
Joe Orton wrote:

> In the model where you have S allocated out of P, let's presume we also 
> have a subpool Q.
> If for some reason specific to the design of your application, you need 
> S to be closed before the destruction of Q, you can register a cleanup 
> against Q which does calls apr_socket_close(S).  That will give defined 
> behaviour.
> The pre_cleanup stuff seems to be entirely redundant in this regard.  I 
> don't understand the motivation for it in the first place, to be honest.

   I'm coming in late to this discussion, and I'm afraid I'm also
going to bail out early, so I apologize if my comments are not
relevant or applicable.

   The particular issue I'd like to see addressed with any kind of
"pre-cleanup" of APR pools is the one we work around today in mod_dbd.
That's a concrete example of a difficult hack around the behaviour
of APR pools and reslists.

   Specifically, suppose you pass pool P to the reslist creation function.
In our constructor for each resource in the list, suppose we need to
allocate a private structure; this is pretty typical.  If you allocate
out of P, you leak memory until the reslist itself is destroyed -- not
good for a reslist which lasts for a long time, e.g., a reslist of DB
connections in httpd.

   OK, so let's create a sub-pool Q in the constructor and allocate
a per-resource structure out of that.  In the destructor we'll
close our DB connection, then destroy Q.

   Here's the rub, though.  Note that the reslist's own creation function
registered a cleanup for the reslist on P.  That reslist cleanup function
begins by calling our destructor function for every resource, which
makes sense.

   But now we can't let P be destroyed, because what it will do first
is destroy all its sub-pools (all the Qs we've created for each resource).
Then it will call the cleanups for P, including the reslist's cleanup,
which will invoke our destructor on each resource, which will now
attempt to destroy each Q a second time.  This was the cause of PR 39985
which took a lot of effort to solve:


   See also the notes in mod_dbd.c in dbd_setup() and the workaround
involving the destructor dbd_destruct(), a flag variable, and
a second cleanup function dbd_destroy() registered on the parent pool
(i.e., P) after the reslist is created.

   After resolving that bug Mladen and I had an exchange regarding
the possibility of a pre-cleanup which would allow reslist to register
a pre-cleanup function that dealt with destroying all the resources,
and a post-cleanup function that did any remaining work on before P
was destroyed.

   Then destroying P would result in first the reslist's pre-cleanup
running (which would call our destructor for each resource, which would
close the DB connection and destroy each resource's Q), then destroying
any remaining sub-pools (but not our Qs since our destructor's already
taken care of them), and finally calling the reslist's usual cleanup,
which might destroy its internal mutexes or whatever before P is destroyed.

   Mladen first described this to me back in the aftermath of dealing
with PR 39985, referencing a previous description of his from mid-2006:


   If this is what's under discussion, please do consider the pre-cleanup
idea carefully, as it would make life a lot easier when handling
reslists of resources with private data structures.  If not, I apologize
again for wasting everyone's time.  Thanks,


GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B

View raw message