Stefan Fuhrmann <stef...@apache.org> writes:
> Thanks, it would be nice if could do something within Subversion because
> httpd and apr patches may take a while to trickle down into releases.
> Meanwhile, I posted a patch for mod_dav on the httpd dev list.
I took a look at what's happening, and I think the root cause of the problem
is that during a PROPFIND walk, we use the request pool for allocations that
happen per every walked entry. This occurs when mod_dav_svn/repos.c:do_walk()
calls dav_propfind_walker() with dav_walk_resource.pool pointing to the request
pool.
Here is what we could do about it:
(1) PROPFIND walker in do_walk() could use an iterpool when preparing the
dav_walk_resource for dav_propfind_walker().
(2) The mod_dav's dav_open_propdb() function could use resource->pool when
creating the subpool. In this particular case, it's not necessary to
solve the long-standing problem with the lifetime of that subpool, and,
since opening a propdb requires a dav_resource, doing so wouldn't violate
propdb's lifetime guarantees.
I prepared two patches that target mod_dav and mod_dav_svn. Together, they
solve the memory usage problem in my experiments.
I plan on posting the patch for mod_dav to the corresponding thread [1] in
<d...@httpd.apache.org> — hopefully, tomorrow.
[1]
https://mail-archives.apache.org/mod_mbox/httpd-dev/201512.mbox/%3C5678958E.20608%40apache.org%3E
Regards,
Evgeny Kotkov
Index: modules/dav/main/props.c
===================================================================
--- modules/dav/main/props.c (revision 1721415)
+++ modules/dav/main/props.c (working copy)
@@ -524,7 +524,7 @@ DAV_DECLARE(dav_error *)dav_open_propdb(request_re
apr_array_header_t * ns_xlate,
dav_propdb **p_propdb)
{
- dav_propdb *propdb = apr_pcalloc(r->pool, sizeof(*propdb));
+ dav_propdb *propdb = apr_pcalloc(resource->pool, sizeof(*propdb));
*p_propdb = NULL;
@@ -537,7 +537,7 @@ DAV_DECLARE(dav_error *)dav_open_propdb(request_re
#endif
propdb->r = r;
- apr_pool_create(&propdb->p, r->pool);
+ apr_pool_create(&propdb->p, resource->pool);
propdb->resource = resource;
propdb->ns_xlate = ns_xlate;
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c (revision 1721415)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -4335,6 +4335,9 @@ do_walk(walker_ctx_t *ctx,
and this is the invocation for the collection. Alternatively, this is
the first [and only] entry to do_walk() for a member resource, so
this will be the invocation for the member. */
+ ctx->wres.pool = scratch_pool;
+ ctx->res.pool = scratch_pool;
+
err = (*params->func)(&ctx->wres,
isdir ? DAV_CALLTYPE_COLLECTION : DAV_CALLTYPE_MEMBER);
if (err != NULL)
@@ -4445,6 +4448,9 @@ do_walk(walker_ctx_t *ctx,
if (dirent->kind == svn_node_file)
{
+ ctx->wres.pool = iterpool;
+ ctx->res.pool = iterpool;
+
err = (*params->func)(&ctx->wres, DAV_CALLTYPE_MEMBER);
if (err != NULL)
{
