[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r15391 - trunk/subversion/svnadmin

From: <kfogel_at_collab.net>
Date: 2005-07-26 22:50:45 CEST

jerenkrantz@tigris.org writes:
> Log:
> Fix a memory leak when performing a long-running svnadmin load or dump.
>
> Temporary allocation for svn_cmdline_fputs is the global pool which is never
> cleared. Therefore, after around 190k commits, we run out of memory.
>
> A user can work around this problem by passing -q to svnadmin.
>
> * subversion/svnadmin/main.c
> (recode_write): clear the pool before each usage.
> (subcommand_dump, subcommand_load): use a sub-pool instead.
>
>
> Modified: trunk/subversion/svnadmin/main.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/svnadmin/main.c?rev=15391&p1=trunk/subversion/svnadmin/main.c&p2=trunk/subversion/svnadmin/main.c&r1=15390&r2=15391
> ==============================================================================
> --- trunk/subversion/svnadmin/main.c (original)
> +++ trunk/subversion/svnadmin/main.c Fri Jul 22 05:56:50 2005
> @@ -559,6 +559,7 @@
> apr_size_t *len)
> {
> struct recode_write_baton *rwb = baton;
> + svn_pool_clear(rwb->pool);
> return svn_cmdline_fputs (data, rwb->out, rwb->pool);
> }
>
> @@ -610,7 +611,7 @@
> if (! opt_state->quiet)
> {
> stderr_stream = svn_stream_create (&stderr_stream_rwb, pool);
> - stderr_stream_rwb.pool = pool;
> + stderr_stream_rwb.pool = svn_pool_create(pool);
> stderr_stream_rwb.out = stderr;
> svn_stream_set_write (stderr_stream, recode_write);
> }
> @@ -673,7 +674,7 @@
> if (! opt_state->quiet)
> {
> stdout_stream = svn_stream_create (&stdout_stream_rwb, pool);
> - stdout_stream_rwb.pool = pool;
> + stdout_stream_rwb.pool = svn_pool_create(pool);
> stdout_stream_rwb.out = stdout;
> svn_stream_set_write (stdout_stream, recode_write);
> }

When we use this method of achieving a loop pool, then we should drop
documentation hints in the appropriate places:

   - recode_write() should state that it may clear BATON->pool

   - 'struct recode_write_baton' should document that its pool field
      should only be used for temporary allocation within a single
      call (or something like that -- I haven't come up with great
      phrasing here, but you get the idea).

(Too bad we didn't make 'svn_write_fn_t' take a pool, of course, but
that's water under the bridge.)

Also, though I quake to say it given recent threads, you've got the
no-space-before-paren thang goin' on :-). If you're fixing up the
documentation anyway, you might want to fix that, but no big deal.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jul 26 23:43:39 2005

This is an archived mail posted to the Subversion Dev mailing list.