You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Hudson <gh...@MIT.EDU> on 2005/01/22 17:05:52 UTC

Re: svn commit: r12801 - in trunk/subversion: include libsvn_client libsvn_ra libsvn_ra_dav libsvn_ra_local libsvn_ra_svn

On Sat, 2005-01-22 at 02:33, kfogel@collab.net wrote:
> This doc string says that RA requests will "continue to use @a pool
> for memory allocation", referring the pool parameter above, which was
> passed to svn_ra_open() and presumably is preserved in the session
> object.
> 
> But this seems to contradict the doc strings of the svn_ra_* functions
> themselves.  For examples:

Ha.  An interesting argument against trying to document our pool policy
in every docstring: it's too hard to say it succinctly enough to say it
over and over again.

What the caller needs to know is that the lifetime of the returned
ra-session object is governed by the passed-in pool.  So, we shouldn't
try to document any more of that.

The callee will use the ra-session pool (as opposed to the temporary
pool) every time it needs to allocate memory in order to mutate the
ra-session object.  For instance, if the ra_svn client needed to
reconnect to the server (say, because we implement an ra_lib->retarget
and the svnserve doesn't support retargeting), that would likely require
allocating memory with the ra-session pool.

This rule is part of our global pool-usage conventions; its simplest
manifestation is in functions like svn_stringbuf_appendcstr().

> If that initial pool is used for much beyond the allocation of the
> session object, then won't we have a memory leak problem when ra
> functions are called repeatedly in loops?  Because even if the caller
> dutifully passes a reuseable pool to each svn_ra_foo() call, some
> allocation would still be happening in that original pool, beyond the

That depends on when the implementation chooses to allocate new memory. 
A loop of svn_stringbuf_setempty() and svn_stringbuf_appendcstr() would
appear to have a memory leak because svn_stringbuf_appendcstr()
potentially allocates memory in the stringbuf pool on each call, but in
reality the stringbuf will never use more than a constant factor times
the maximum size of the string.

For an ra-session object it's harder to define a theoretical limit like
that, but we're unlikely to see a practical problem if we use the
ra-session pool as sparingly as possible.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r12801 - in trunk/subversion: include libsvn_client libsvn_ra libsvn_ra_dav libsvn_ra_local libsvn_ra_svn

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2005-01-23 at 15:10, Peter N. Lundblad wrote:
> > What the caller needs to know is that the lifetime of the returned
> > ra-session object is governed by the passed-in pool.  So, we shouldn't
> > try to document any more of that.
[Oops.  "... any more than that".]

> I am going to document the above and that the pool may be used by the RA
> implementation for session-related data even during following function
> calls as some kind of compromise.

Okay.

> Wouldn't we use a subpool for the internal session in the a case
> like the one examplified above to avoid a memleak?

Yeah, we could.  But we'd still be using the ra-session pool in some
sense.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r12801 - in trunk/subversion: include libsvn_client libsvn_ra libsvn_ra_dav libsvn_ra_local libsvn_ra_svn

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sat, 22 Jan 2005, Greg Hudson wrote:

> On Sat, 2005-01-22 at 02:33, kfogel@collab.net wrote:
> > This doc string says that RA requests will "continue to use @a pool
> > for memory allocation", referring the pool parameter above, which was
> > passed to svn_ra_open() and presumably is preserved in the session
> > object.
> >
> > But this seems to contradict the doc strings of the svn_ra_* functions
> > themselves.  For examples:
>
> Ha.  An interesting argument against trying to document our pool policy
> in every docstring: it's too hard to say it succinctly enough to say it
> over and over again.
>
> What the caller needs to know is that the lifetime of the returned
> ra-session object is governed by the passed-in pool.  So, we shouldn't
> try to document any more of that.
>
I am going to document the above and that the pool may be used by the RA
implementation for session-related data even during following function
calls as some kind of compromise.

> The callee will use the ra-session pool (as opposed to the temporary
> pool) every time it needs to allocate memory in order to mutate the
> ra-session object.  For instance, if the ra_svn client needed to
> reconnect to the server (say, because we implement an ra_lib->retarget
> and the svnserve doesn't support retargeting), that would likely require
> allocating memory with the ra-session pool.
>
> > If that initial pool is used for much beyond the allocation of the
> > session object, then won't we have a memory leak problem when ra
> > functions are called repeatedly in loops?  Because even if the caller
> > dutifully passes a reuseable pool to each svn_ra_foo() call, some
> > allocation would still be happening in that original pool, beyond the
>
> That depends on when the implementation chooses to allocate new memory.
> A loop of svn_stringbuf_setempty() and svn_stringbuf_appendcstr() would
> appear to have a memory leak because svn_stringbuf_appendcstr()
> potentially allocates memory in the stringbuf pool on each call, but in
> reality the stringbuf will never use more than a constant factor times
> the maximum size of the string.
>
> For an ra-session object it's harder to define a theoretical limit like
> that, but we're unlikely to see a practical problem if we use the
> ra-session pool as sparingly as possible.
>
Wouldn't we use a subpool for the internal session in the a case
like the one examplified above to avoid a memleak?

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org