We learned a lot about pool handling while writing Subversion, after I wrote that mod_dav code. There are definite some improvements to be made. I'm not surprised that a propfind can go nuts like that ... I'll review the change and take a look generally.
h/t to DanielR for the pointer to this thread. On Tue, Sep 18, 2018 at 8:04 AM Ruediger Pluem <rpl...@apache.org> wrote: > Pools are very tricky in mod_dav. Hence additional eyeballs are very much > welcome here. > As I only did testing with mod_dav_fs I would be keen to know if things > still work with Subversion. > So if someone from the Subversion guys is listening here: Having this > tested with Subversion would be very welcome :-). > > Regards > > Rüdiger > > On 09/18/2018 02:58 PM, rpl...@apache.org wrote: > > Author: rpluem > > Date: Tue Sep 18 12:58:57 2018 > > New Revision: 1841225 > > > > URL: http://svn.apache.org/viewvc?rev=1841225&view=rev > > Log: > > * Doing a PROPFIND on a large collection e.g. 50.000 elements can easily > > consume 1 GB of memory as the subrequests and propdb pools are not > > destroyed and cleared after each element was handled. > > Do this now. There is one case in dav_get_props where elem->priv > > lives longer then the propdb pool. In this case allocate from r->pool. > > Furthermore also recycle propdb's which allows to clear the propdb's > > pools instead of destroying them and creating them again. > > > > Modified: > > httpd/httpd/trunk/modules/dav/main/props.c > > > > Modified: httpd/httpd/trunk/modules/dav/main/props.c > > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/props.c?rev=1841225&r1=1841224&r2=1841225&view=diff > > > ============================================================================== > > --- httpd/httpd/trunk/modules/dav/main/props.c (original) > > +++ httpd/httpd/trunk/modules/dav/main/props.c Tue Sep 18 12:58:57 2018 > > @@ -524,7 +524,21 @@ DAV_DECLARE(dav_error *)dav_open_propdb( > > apr_array_header_t * ns_xlate, > > dav_propdb **p_propdb) > > { > > - dav_propdb *propdb = apr_pcalloc(r->pool, sizeof(*propdb)); > > + dav_propdb *propdb = NULL; > > + /* > > + * Check if we have tucked away a previous propdb and reuse it. > > + * Otherwise create a new one and tuck it away > > + */ > > + apr_pool_userdata_get((void **)&propdb, "propdb", r->pool); > > + if (!propdb) { > > + propdb = apr_pcalloc(r->pool, sizeof(*propdb)); > > + apr_pool_userdata_setn(propdb, "propdb", NULL, r->pool); > > + apr_pool_create(&propdb->p, r->pool); > > + } > > + else { > > + /* Play safe and clear the pool of the reused probdb */ > > + apr_pool_clear(propdb->p); > > + } > > > > *p_propdb = NULL; > > > > @@ -537,7 +551,6 @@ DAV_DECLARE(dav_error *)dav_open_propdb( > > #endif > > > > propdb->r = r; > > - apr_pool_create(&propdb->p, r->pool); > > propdb->resource = resource; > > propdb->ns_xlate = ns_xlate; > > > > @@ -562,10 +575,12 @@ DAV_DECLARE(void) dav_close_propdb(dav_p > > (*propdb->db_hooks->close)(propdb->db); > > } > > > > - /* Currently, mod_dav's pool usage doesn't allow clearing this > pool. */ > > -#if 0 > > - apr_pool_destroy(propdb->p); > > -#endif > > + if (propdb->subreq) { > > + ap_destroy_sub_req(propdb->subreq); > > + propdb->subreq = NULL; > > + } > > + > > + apr_pool_clear(propdb->p); > > } > > > > DAV_DECLARE(dav_get_props_result) dav_get_allprops(dav_propdb *propdb, > > @@ -815,7 +830,8 @@ DAV_DECLARE(dav_get_props_result) dav_ge > > */ > > > > if (elem->priv == NULL) { > > - elem->priv = apr_pcalloc(propdb->p, sizeof(*priv)); > > + /* elem->priv outlives propdb->p. Hence use the request > pool */ > > + elem->priv = apr_pcalloc(propdb->r->pool, sizeof(*priv)); > > } > > priv = elem->priv; > > > > > > > > >
