You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@gmail.com> on 2010/05/12 21:30:29 UTC
Re: svn commit: r942897 - in /httpd/httpd/trunk: CHANGES
server/mpm/prefork/prefork.c
On Wed, May 12, 2010 at 4:24 AM, Joe Orton <jo...@redhat.com> wrote:
> On Mon, May 10, 2010 at 08:47:59PM -0000, Jeff Trawick wrote:
>> --- httpd/httpd/trunk/server/mpm/prefork/prefork.c (original)
>> +++ httpd/httpd/trunk/server/mpm/prefork/prefork.c Mon May 10 20:47:59 2010
>> @@ -549,12 +549,6 @@ static void child_main(int child_num_arg
>> conn_rec *current_conn;
>> void *csd;
>>
>> - /*
>> - * (Re)initialize this child to a pre-connection state.
>> - */
>> -
>> - apr_pool_clear(ptrans);
>> -
>
> ptrans will not be cleared between loop iterations in the case where
> accept fails (the "continue" path) - isn't that a potential memory leak?
If the accept() call itself fails, I don't think it will leak with
current code; if something in APR after accept() fails (probably
should-not-occur), then yes.
The multiple-calls-to-pool_clear solution is definitely safer.
(Thanks!)
Re: svn commit: r942897 - in /httpd/httpd/trunk: CHANGES
server/mpm/prefork/prefork.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, May 19, 2010 at 5:26 PM, Joe Orton <jo...@redhat.com> wrote:
> On Wed, May 12, 2010 at 03:30:29PM -0400, Jeff Trawick wrote:
>> The multiple-calls-to-pool_clear solution is definitely safer.
>
> OK, so now I reviewed that too ;) The only difference between:
>
> apr_pool_clear(ptrans);
> apr_pool_destroy(pchild);
>
> and simply:
>
> apr_pool_destroy(pchild);
>
> (given that ptrans is a child of ptrans and _destroy runs cleanups in
> child pools)
>
> is that it hides problems in cleanup ordering/pool nesting.
hmmm, definitely a negative characteristic... too bad it is
(apparently) rolled up in the following issue
> I think PR 43857 is about the reslist cleanup issue in APR-util, r677505
> and all that. Not sure what the status of that is.
main, initial discussion ends here:
http://marc.info/?l=apr-dev&m=121728874016894&w=3
meanwhile, unreleased apr trunk and unreleased apr-util 1.4.x have
APU_DECLARE(void) apr_reslist_cleanup_order_set(apr_reslist_t *rl,
apr_uint32_t mode)
{
apr_pool_cleanup_kill(rl->pool, rl, reslist_cleanup);
if (mode == APR_RESLIST_CLEANUP_FIRST)
apr_pool_pre_cleanup_register(rl->pool, rl, reslist_cleanup);
else
apr_pool_cleanup_register(rl->pool, rl, reslist_cleanup,
apr_pool_cleanup_null);
}
and apr_memcache uses that
discussion of backporting seems to have stopped at this point:
http://marc.info/?l=apr-dev&m=121661852120171&w=3
Re: svn commit: r942897 - in /httpd/httpd/trunk: CHANGES
server/mpm/prefork/prefork.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, May 19, 2010 at 5:26 PM, Joe Orton <jo...@redhat.com> wrote:
> On Wed, May 12, 2010 at 03:30:29PM -0400, Jeff Trawick wrote:
>> The multiple-calls-to-pool_clear solution is definitely safer.
>
> OK, so now I reviewed that too ;) The only difference between:
>
> apr_pool_clear(ptrans);
> apr_pool_destroy(pchild);
>
> and simply:
>
> apr_pool_destroy(pchild);
>
> (given that ptrans is a child of ptrans and _destroy runs cleanups in
> child pools)
>
> is that it hides problems in cleanup ordering/pool nesting.
hmmm, definitely a negative characteristic... too bad it is
(apparently) rolled up in the following issue
> I think PR 43857 is about the reslist cleanup issue in APR-util, r677505
> and all that. Not sure what the status of that is.
main, initial discussion ends here:
http://marc.info/?l=apr-dev&m=121728874016894&w=3
meanwhile, unreleased apr trunk and unreleased apr-util 1.4.x have
APU_DECLARE(void) apr_reslist_cleanup_order_set(apr_reslist_t *rl,
apr_uint32_t mode)
{
apr_pool_cleanup_kill(rl->pool, rl, reslist_cleanup);
if (mode == APR_RESLIST_CLEANUP_FIRST)
apr_pool_pre_cleanup_register(rl->pool, rl, reslist_cleanup);
else
apr_pool_cleanup_register(rl->pool, rl, reslist_cleanup,
apr_pool_cleanup_null);
}
and apr_memcache uses that
discussion of backporting seems to have stopped at this point:
http://marc.info/?l=apr-dev&m=121661852120171&w=3
Re: svn commit: r942897 - in /httpd/httpd/trunk: CHANGES
server/mpm/prefork/prefork.c
Posted by Joe Orton <jo...@redhat.com>.
On Wed, May 12, 2010 at 03:30:29PM -0400, Jeff Trawick wrote:
> The multiple-calls-to-pool_clear solution is definitely safer.
OK, so now I reviewed that too ;) The only difference between:
apr_pool_clear(ptrans);
apr_pool_destroy(pchild);
and simply:
apr_pool_destroy(pchild);
(given that ptrans is a child of ptrans and _destroy runs cleanups in
child pools)
is that it hides problems in cleanup ordering/pool nesting.
I think PR 43857 is about the reslist cleanup issue in APR-util, r677505
and all that. Not sure what the status of that is.
Regards, Joe
Re: svn commit: r942897 - in /httpd/httpd/trunk: CHANGES
server/mpm/prefork/prefork.c
Posted by Joe Orton <jo...@redhat.com>.
On Wed, May 12, 2010 at 03:30:29PM -0400, Jeff Trawick wrote:
> The multiple-calls-to-pool_clear solution is definitely safer.
OK, so now I reviewed that too ;) The only difference between:
apr_pool_clear(ptrans);
apr_pool_destroy(pchild);
and simply:
apr_pool_destroy(pchild);
(given that ptrans is a child of ptrans and _destroy runs cleanups in
child pools)
is that it hides problems in cleanup ordering/pool nesting.
I think PR 43857 is about the reslist cleanup issue in APR-util, r677505
and all that. Not sure what the status of that is.
Regards, Joe