Philip Martin <philip@codematters.co.uk> writes:
> $ valgrind -q svnadmin dump empty-space2/ > /dev/null
> * Dumped revision 0.
> ==1662== Invalid write of size 4
> ==1662== at 0x402577D2: uncache_node_revision (../svn/subversion/libsvn_fs/dag.c:133)
> ==1662== by 0x4026130F: do_retry (../svn/subversion/libsvn_fs/trail.c:192)
> ==1662== by 0x40261456: svn_fs__retry_txn (../svn/subversion/libsvn_fs/trail.c:245)
> ==1662== by 0x40262A73: svn_fs_node_id (../svn/subversion/libsvn_fs/tree.c:1125)
> ==1662== Address 0x42103C78 is 16 bytes inside a block of size 24 free'd
> ==1662== at 0x40027C2A: free (vg_replace_malloc.c:220)
> ==1662== by 0x403A98C2: pool_clear_debug (apr_pools.c:1425)
> ==1662== by 0x403A994E: apr_pool_clear_debug (apr_pools.c:1460)
> ==1662== by 0x40261870: dag_node_cache_set (../svn/subversion/libsvn_fs/tree.c:317)
> ==1662==
> ==1662== Invalid write of size 4
> ==1662== at 0x402577D2: uncache_node_revision (../svn/subversion/libsvn_fs/dag.c:133)
> ==1662== by 0x4026130F: do_retry (../svn/subversion/libsvn_fs/trail.c:192)
> ==1662== by 0x40261456: svn_fs__retry_txn (../svn/subversion/libsvn_fs/trail.c:245)
> ==1662== by 0x402630E1: svn_fs_node_proplist (../svn/subversion/libsvn_fs/tree.c:1375)
> ==1662== Address 0x42102060 is 16 bytes inside a block of size 24 free'd
> ==1662== at 0x40027C2A: free (vg_replace_malloc.c:220)
> ==1662== by 0x403A98C2: pool_clear_debug (apr_pools.c:1425)
> ==1662== by 0x403A994E: apr_pool_clear_debug (apr_pools.c:1460)
> ==1662== by 0x40261870: dag_node_cache_set (../svn/subversion/libsvn_fs/tree.c:317)
>
> Yikes! Is that a bug in the libsvn_fs node cache?
I would not be surprised if it is. Take a look at
dag_node_cache_set() and you'll see a comment I left about a section
of that code being broken for reasons I could not determine. I made
that code abort() and made sure it didn't get called, but I bet there
is some other similar badness that I never came across.
uncache_node_revision() is a registed cleanup on the trail->pool in
which the node revision was read. But we should always be dup'ing the
dag_node_t structures (and *explictly* leaving their ->node pointers
NULL) into the revision root's ->pool when caching. So, a revision
root cache should never contain dag_node_t's with non-NULL
node-revision structures.
Hmm, but I guess I can envision a scenario in which:
1. a cached dag_node_t is passed to the DAG layer, doesn't have
node-revision attached to it, and so gets it populated in
trail->pool (and an uncache_node_revision cleanup is
registered).
2. due to cache rotation, the whole dag_node_t structure is
free()d.
3. later, the trail shuts down and the uncache_node_revision() is
triggered, except that the memory address for that node-revision
structure happens to sit in the middle of a free()d dag_node_t.
The complexity of this system might be a little too much to deal with.
Originally, I had planned to cache only the svn_fs_id_t's, not the
whole dag_node_t structures. Perhaps I should return to that plan
just to be safe.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Oct 26 17:35:52 2003