Re: svn commit: r989108 - /subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c

philip_at_apache.org writes:

> Author: philip
> Date: Wed Aug 25 13:53:27 2010
> New Revision: 989108
>
> URL: http://svn.apache.org/viewvc?rev=989108&view=rev
> Log:
> Stop writing to pool memory after the pool has been cleared, something
> that started to happen when svn_ra_open4 introduced a session pool. The
> problem was identified by valgrind.
>
> * subversion/libsvn_ra/ra_plugin.c
> (get_username): Use the session pool to allocate the svn_fs_access_t and
> to setup the pool cleanup.
>
> Modified:
> subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
>
> Modified: subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c?rev=989108&r1=989107&r2=989108&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_local/ra_plugin.c Wed Aug 25 13:53:27 2010
> @@ -115,12 +115,12 @@ get_username(svn_ra_session_t *session,
> if (*sess->username)
> {
> SVN_ERR(svn_fs_create_access(&access_ctx, sess->username,
> - pool));
> + session->pool));
> SVN_ERR(svn_fs_set_access(sess->fs, access_ctx));
>
> /* Make sure this context is disassociated when the pool gets
> destroyed. */
> - apr_pool_cleanup_register(pool, sess->fs, cleanup_access,
> + apr_pool_cleanup_register(session->pool, sess->fs, cleanup_access,
> apr_pool_cleanup_null);
> }

Mike, this was necessary to fix this problem:

$ svnadmin create repo
$ mkdir greek
$ valgrind -q subversion/svn/.libs/lt-svn import -mm greek file://`pwd`/repo
==6804== Invalid write of size 8
==6804== at 0x6C2A5BF: svn_fs_set_access (access.c:58)
==6804== by 0x67E4325: cleanup_access (ra_plugin.c:56)
==6804== by 0x5E3629A: run_cleanups (apr_pools.c:2071)
==6804== by 0x5E35398: pool_clear_debug (apr_pools.c:1409)
==6804== by 0x5E35599: pool_destroy_debug (apr_pools.c:1494)
==6804== by 0x5E3566D: apr_pool_destroy_debug (apr_pools.c:1536)
==6804== by 0x4E3EA13: svn_client_import4 (commit.c:825)
==6804== by 0x40D982: svn_cl__import (import-cmd.c:119)
==6804== by 0x4156E8: main (main.c:2329)
==6804== Address 0xa8ddc90 is 40 bytes inside a block of size 64 free'd
==6804== at 0x4C2130F: free (vg_replace_malloc.c:323)
==6804== by 0x5E35469: pool_clear_debug (apr_pools.c:1432)
==6804== by 0x5E35599: pool_destroy_debug (apr_pools.c:1494)
==6804== by 0x5E3537E: pool_clear_debug (apr_pools.c:1406)
==6804== by 0x5E35599: pool_destroy_debug (apr_pools.c:1494)
==6804== by 0x5E3566D: apr_pool_destroy_debug (apr_pools.c:1536)
==6804== by 0x4E3EA13: svn_client_import4 (commit.c:825)
==6804== by 0x40D982: svn_cl__import (import-cmd.c:119)
==6804== by 0x4156E8: main (main.c:2329)

It was caused by r965856 where svn_ra_open4 creates a pool and
allocates the returned session inside that pool. Ususally our
allocation functions allocate directly in the supplied pool. Why the
change?

-- 
Philip