Chris Darroch
Mon, 21 Jul 2008 10:12:19 -0700
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: https://issues.apache.org/bugzilla/show_bug.cgi?id=39985 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: [EMAIL PROTECTED] [EMAIL PROTECTED] 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, Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B