On 25 February 2015 at 17:40, Branko Čibej <br...@wandisco.com> wrote: > On 25.02.2015 15:23, i...@apache.org wrote: >> Author: ivan >> Date: Wed Feb 25 14:23:00 2015 >> New Revision: 1662222 >> >> URL: http://svn.apache.org/r1662222 >> Log: >> On the 'reuse-ra-session' branch: Resolve memory leak. >> >> * subversion/libsvn_client/ra_cache.c >> (): Include "svn_pools.h". >> >> (cache_entry_t): Add SESSION_POOL member to have an option to destroy >> CACHE_ENTRY structure itself. >> >> (close_ra_session, remove_inactive_entry): Destroy SESSION_POOL >> instead of closing RA session -- we have other resources allocated >> for RA session, like RA callback baton, cache_entry etc. >> >> (open_new_session): New helper, factored out from >> svn_client__ra_cache_open_session(). >> >> (svn_client__ra_cache_open_session): Create separate subpool for >> each CACHE_ENTRY and destroy it on error. > > So ... from this point on, we create *two* subpools for each session > (svn_ra_open creates one, and with the above change, we create the > other). That seems wasteful. > That's correct. But RA session relatively expensive object (it maintain TCP connections etc) and one extra subpool should not be big issue IMO.
> I'd prefer to have a way to tell svn_ra_open to /not/ create a subpool > for the session, perhaps by exposing a private ra-open function where we > can pass in the actual session pool, and to keep svn_ra__close. > We still need separate subpool to allocate CACHE_ENTRY itself, callbacks baton etc. And inventing private API to just save one subpool looks like a bad idea for me. I'd prefer to drop SESSPOOL in ra-loader.c if we consider that two subpools per RA session is serious problem: it was added in r984280 when HTTP redirects was implemented. I think that the caller should be responsible to clear pool if it's going to follow redirects. -- Ivan Zhakov
