Roderich Schupp <roderich.schupp_at_gmail.com> writes:
> So short_circuit path authorization is to blame.
> I think the explanation is:
>
> If we don't do short_circuit, then (cf. mod_dav_svn/authz.c)
>
> ap_sub_req_method_uri("GET", ...)
>
> is called foreach changed path. This runs with a freshly created
> request_rec rsub
> where rsub->pool is a sub-pool of the main r->pool. When the sub
> request returns,
> rsub and rsub->pool are destroyed. The actual work is done in
> svn_repos_authz_check_access which gets passed rsub->pool.
>
> But for short_circuit authz, svn_repos_authz_check_access is called from
> subreq_bypass using the main request r->pool (cf.
> mod_authz_svn/mod_authz_svn.c).
> I.e. the memory used by svn_repos_authz_check_access accumulates over
> the whole log-report HTTP request.
>
> To fix, use a sub-pool in subreq_bypass when calling
> svn_repos_authz_check_access.
Yes, that is a problem. We generally try to avoid having functions
allocate pools unless they are iterating, otherwise we make it the
callers responsibility to allocate/clear the pools. In this case the
loop happens in detect_changed which is correctly clearing an iteration
pool and passing it to dav_svn__allow_read. The problem is that when
dav_svn__allow_read calls allow_read_bypass (which is subreq_bypass)
this pool can't be passed and the request pool is used.
I'm not sure how best to solve this. Your patch certainly is one
solution. Ideally we would be using the iteration pool in
detect_changed and passing it on to subreq_bypass but I'm not sure how
to achieve that. Any of our mod_dav_svn experts care to comment?
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Received on 2012-09-18 21:12:32 CEST