AW: svn commit: r1841225 - /httpd/httpd/trunk/modules/dav/main/props.c

[Plüm , Rüdiger , Vodafone Group]

C2 General

> -----Ursprüngliche Nachricht-----
> Von: Joe Orton <jor...@redhat.com>
> Gesendet: Dienstag, 25. Juni 2019 09:49
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1841225 -
> /httpd/httpd/trunk/modules/dav/main/props.c
> 
> On Mon, Jun 24, 2019 at 09:54:21PM +0200, Ruediger Pluem wrote:
> > On 06/24/2019 07:04 PM, Joe Orton wrote:
> > > #11 0x00007f8110ce6448 in dav_do_prop_subreq (propdb=0x100cfc0) at
> props.c:331
> >
> > What if you go here and do a
> >
> > dump_all_pools propdb->p
> >
> > and do it again when you hit the next sbrk and the next one and so on?
> Something suspicious?
> 
> Well I did this and immediately noticed something suspicious on the
> previous line, e_uri is allocated from resource->pool (=r->pool) rather
> than propbb->p, so, yet again the precision accuracy of your insights is

Very good catch. Great that you found it.

> beyond belief Ruediger :) :)
> 
> This fixed a significant leak, but r->pool is still growing.  I think
> the remaining problem is use of r->pool in dav_fs_walker, i.e. is
> mod_dav_fs specific.

Do you think it is now good enough to commit at least as a first step?

> 
> Unless I am missing something the apr_dir_open/apr_dir_read API design
> is always going to have memory use proportional to directory size
> because apr_dir_read() duplicates the filename into the directory's
> pool.  We need an apr_dir_pread() or whatever which takes a pool
> argument. :(

In order to fix this leak, I agree that we would need this.
The pool is also used for apr_stat inside apr_dir_read, but I fail to see
how it consumes memory.

Another related question: Do you think it would be beneficial if we replace
the apr_psprintf with apr_pstrcat in props.c as long as they only concat 
strings without
further formatting?


Regards

Rüdiger