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
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.
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. :(
Index: modules/dav/main/mod_dav.c
===================================================================
--- modules/dav/main/mod_dav.c (revision 1861995)
+++ modules/dav/main/mod_dav.c (working copy)
@@ -563,6 +563,7 @@
dav_begin_multistatus(bb, r, status, namespaces);
apr_pool_create(&subpool, r->pool);
+ apr_pool_tag(subpool, "mod_dav-multistatus");
for (; first != NULL; first = first->next) {
apr_pool_clear(subpool);
@@ -2027,8 +2028,9 @@
** Note: we cast to lose the "const". The propdb won't try to change
** the resource, however, since we are opening readonly.
*/
- err = dav_open_propdb(ctx->r, ctx->w.lockdb, wres->resource, 1,
- ctx->doc ? ctx->doc->namespaces : NULL, &propdb);
+ err = dav_popen_propdb(ctx->scratchpool,
+ ctx->r, ctx->w.lockdb, wres->resource, 1,
+ ctx->doc ? ctx->doc->namespaces : NULL, &propdb);
if (err != NULL) {
/* ### do something with err! */
Index: modules/dav/main/mod_dav.h
===================================================================
--- modules/dav/main/mod_dav.h (revision 1861995)
+++ modules/dav/main/mod_dav.h (working copy)
@@ -1596,6 +1596,16 @@
apr_array_header_t *ns_xlate,
dav_propdb **propdb);
+DAV_DECLARE(dav_error *) dav_popen_propdb(
+ apr_pool_t *p,
+ request_rec *r,
+ dav_lockdb *lockdb,
+ const dav_resource *resource,
+ int ro,
+ apr_array_header_t *ns_xlate,
+ dav_propdb **propdb);
+
+
DAV_DECLARE(void) dav_close_propdb(dav_propdb *db);
DAV_DECLARE(dav_get_props_result) dav_get_props(
Index: modules/dav/main/props.c
===================================================================
--- modules/dav/main/props.c (revision 1861995)
+++ modules/dav/main/props.c (working copy)
@@ -323,7 +323,7 @@
{
/* need to escape the uri that's in the resource struct because during
* the property walker it's not encoded. */
- const char *e_uri = ap_escape_uri(propdb->resource->pool,
+ const char *e_uri = ap_escape_uri(propdb->p,
propdb->resource->uri);
/* perform a "GET" on the resource's URI (note that the resource
@@ -524,22 +524,21 @@
apr_array_header_t * ns_xlate,
dav_propdb **p_propdb)
{
+ return dav_popen_propdb(r->pool, r, lockdb, resource, ro, ns_xlate,
p_propdb);
+}
+
+DAV_DECLARE(dav_error *)dav_popen_propdb(apr_pool_t *p,
+ request_rec *r, dav_lockdb *lockdb,
+ const dav_resource *resource,
+ int ro,
+ apr_array_header_t * ns_xlate,
+ dav_propdb **p_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);
- }
+ propdb = apr_pcalloc(p, sizeof(*propdb));
+ propdb->p = p;
+
*p_propdb = NULL;
#if DAV_DEBUG
@@ -579,8 +578,6 @@
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,
