apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: [PATCH] fix cleanups in cleanups
Date Fri, 21 Sep 2001 07:34:52 GMT
On Thu, Sep 20, 2001 at 07:54:22PM -0700, Ryan Bloom wrote:
> On Thursday 20 September 2001 05:48 pm, Greg Stein wrote:
...
> > Calling pop_cleanup() on every iteration is a bit much. Consider the
> > following patch:
> 
> Why is it a bit much?  I just took a quick look at it, it is an if, and three assignments.

Heh. It is a bit much when you consider you consider a thread in October
1999 discussing using NULL rather than ap_null_cleanup:

http://www.humanfactor.com/cgi-bin/cgi-delegate/apache-ML/nh/1999/Oct/0189.html

In that case, an "if" was considered too much :-)

> I would assume that any compiler worth it's salt would inline this function as well.

Very bad assumption. But this isn't the place to discuss compilers.

> This patch also keeps the LIFO behavior, which is important, because it means 
> that it is much less likely that an item allocated out of the pool when the cleanup
> was registered will no longer be there when the cleanup is run.

I am telling you that dependency upon items in the pool is bogus. You cannot
do that. Thus, LIFO means diddly squat.

Consider a case where A inserts its cleanup, expecting to have B present. At
some point between then and the pool cleanup, B goes and removes its
cleanup, then inserts some other cleanup. This new one, B', will now run
*before* A's cleanup.

Gee. Guess what? Your oh-so-needed LIFO behavior has been screwed. B is
*not* present for A to use.

The *only* guarantees you have are between parent and child pools. Within a
pool, you've got nothing.

> > while ((c = p->cleanups) != NULL) {
> >     p->cleanups = NULL;
> >     run_cleanups(c);
> > }
> 
> Which function is this in?  I have looked, but the only place that I can find to
> put this code would be in apr_pool_clear, around the run_cleanups code.

Yes, it would go in apr_pool_clear. And a similar one for the child cleanup
case.

> > Basically, the above code processes the cleanups in batches. Everything
> > that was initially registered is processed, then everything registerd
> > during the first cleanup round, etc.
> 
> This makes it more more likely that a variable in the same pool that was available
> when the cleanup was registered would not be available when your cleanup
> ran.  I would really want to see a performance analysis before we broke that
> behavior.

Broke what behavior? Registering a cleanup in a cleanup doesn't work now.
The new behavior is, "if you register it, I'll run it after I'm done running
these other cleanups."


Stop. Take a breath. Think about it. LIFO doesn't matter.

-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message