Segmentation fault in git-svn

I expect this is really a libsvn bug, but git-svn triggers it, so I'm hoping someone else has run into and solved it, or at least that someone can reproduce it.

If I try to clone the "memcached" public repository with the command line

git-svn clone --branches=branches --trunk=trunk
http://code.sixapart.com/svn/memcached

it cranks along fine until revision 299, then dies with SIGSEGV. If I run it again, it appears to pick up where it left off, then dies again at revision 399, then again at revision 499. (There are fewer than 600 revisions in that repo so it's anyone's guess if it'd die at 599 given the chance.)

This happens on both a Linux box (amd64, FC4, svn version 1.4.3) and my Mac (Intel OS 10.4, svn version 1.2.3 from Fink), so at the very least it's not platform-specific. It also happens periodically on the private svn repository at my company, though not as predictably. On my Mac, I'm using the very latest git code from "master".

I ran the Perl interpreter under gdb to get a stack trace:

#0 get_private (hk=0x8050300000001, id=0x2aaaaf187b2b "SVN") at ne_request.c:273
#1 0x00002aaaaf186983 in parsed_request (sess=0xfbc0e0, method=0x2aaaaf188e19 "PROPFIND", url=0x11e5ab0 "/svn/memcached",
body=0x100ce00 "<?xml version=\"1.0\" encoding=\"utf-8\"?><propfind
xmlns=\"DAV:\"><prop><version-controlled-configuration xmlns=\"DAV:\"/><resourcetype xmlns=\"DAV:\"/><baseline-relative-path xmlns=\"http://subversion.tigris.o"..., body_file=0x0, set_parser=0x2aaaaf1816e0 <set_parser>, elements=0x2aaaaf28c9e0, use_neon_shim=1, validate_compat_cb=0x2aaaaf1811b0 <validate_element>,
startelm_compat_cb=0x2aaaaf181290 <start_element>,
endelm_compat_cb=0x2aaaaf181400 <end_element>, startelm_cb=0, cdata_cb=0, endelm_cb=0, baton=0x7fffff84f630, extra_headers=0x11e5ac0,
status_code=0x0, spool_response=0, pool=0x11e5788) at
subversion/libsvn_ra_dav/util.c:602
#2 0x00002aaaaf187228 in svn_ra_dav__parsed_request_compat (sess=Variable "sess" is not available.
) at subversion/libsvn_ra_dav/util.c:876
#3 0x00002aaaaf1818b8 in svn_ra_dav__get_props (results=0x7fffff84f6d0, sess=0xfbc0e0, url=0x11e5ab0 "/svn/memcached", depth=Variable "depth" is not available.
) at subversion/libsvn_ra_dav/props.c:531
#4 0x00002aaaaf1819b6 in svn_ra_dav__get_props_resource (rsrc=0x7fffff84f7e8, sess=0xfbc0e0, url=0x11e5a80 "/svn/memcached", label=0x0, which_props=0x2aaaaf28c940, pool=0x11e5788)
at subversion/libsvn_ra_dav/props.c:558
#5 0x00002aaaaf181c16 in svn_ra_dav__search_for_starting_props (rsrc=0x7fffff84f7e8, missing_path=0x7fffff84f7e0, sess=0xfbc0e0, url=0xfaefd8 "http://code.sixapart.com/svn/memcached",
pool=0x11e5788) at subversion/libsvn_ra_dav/props.c:667
#6 0x00002aaaaf181e85 in svn_ra_dav__get_baseline_props (bc_relative=0x7fffff84f850, bln_rsrc=0x7fffff84f868, sess=0xfbc0e0, url=Variable "url" is not available.
) at subversion/libsvn_ra_dav/props.c:775
#7 0x00002aaaaf1824a3 in svn_ra_dav__get_baseline_info (is_dir=0x0, bc_url=0x7fffff84f990, bc_relative=0x7fffff84f980, latest_rev=0x0, sess=0xfbc0e0, url=Variable "url" is not available.
)
at subversion/libsvn_ra_dav/props.c:898
#8 0x00002aaaaf17ffe0 in svn_ra_dav__get_log (session=Variable "session" is not available.
) at subversion/libsvn_ra_dav/log.c:430
#9 0x00002aaaaf4c46fa in _wrap_svn_ra_get_log (my_perl=Variable "my_perl" is not available.
) at svn_ra.c:4552
#10 0x0000003bd9e9b67e in Perl_pp_entersub () from /usr/lib64/perl5/5.8.6/x86_64-linux-thread-multi/CORE/libperl.so
#11 0x0000003bd9e7f3cd in Perl_runops_debug () from /usr/lib64/perl5/5.8.6/x86_64-linux-thread-multi/CORE/libperl.so
#12 0x0000003bd9e39b49 in perl_run () from /usr/lib64/perl5/5.8.6/x86_64-linux-thread-multi/CORE/libperl.so
#13 0x0000000000401a01 in main ()

Can anyone else reproduce this?

-Steve

I've also been working with another user off-list the past few days on tracking down on a segmentation fault in git-svn.

I just ran this several (6?) times got a segmentation fault once with Junio's master @ f948792990f82a35bf0c98510e7511ef8acb9cd3.

Trying it again with the same file that segfaulted earlier, it was successful, spooky...

I also tried some other patches (see replies) which I've been testing to fix the other user's problem.

I have definitely not seen this pattern before.

However, the following clone has been problematic at times

git svn clone svn://svn.debian.org/svn/pkg-glibc/glibc-package \
-T trunk -b branches -t tags

I'm running x86, Debian Etch, SVN 1.4.2 from Debian Etch or 1.4.3 from Debian experimental + do_switch patch[1]

Thanks for the stack trace. This is segfaulting at a different place (expected, being an http:// repo while the other is svn://)

[1] - http://svn.haxx.se/dev/archive-2007-01/0936.shtml

Eric Wong

The dirent hash objects returned by get_dir() rely on the temporary pool allocated within our wrapper function itself. Apparently pushing the pool into global storage that was used in its creation does not seem to be enough to avoid it from being garbage-collected; so duplicate that into a plain-old nested hash...

This should not cause data from imports to be corrupted, as get_dir is only used between the fetching of revisions and not the revision data itself. The memory usage done during the actual data fetches (which is checksummed) only relies on newly allocated pools.

Also, put our caches for get_dir and check_path calls directly into the SVN::Ra object so they auto-expire when it is destroyed.

Since calling rev_proplist repeatedly per-revision is no longer needed in git-svn, we do not cache calls to it.

---
 git-svn.perl |   68 +++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 42 insertions(+), 26 deletions(-)

index 7ebd07b..eb3b79c 100755
@@ -1360,7 +1360,7 @@ sub traverse_ignore {
 		}
 	}
 	foreach (sort keys %$dirent) {
-		next if $dirent->{$_}->kind != $SVN::Node::dir;
+		next if $dirent->{$_}->{kind} != $SVN::Node::dir;
 		$self->traverse_ignore($fh, "$path/$_", $r);
 	}
 }

@@ -2855,7 +2855,7 @@ my ($can_do_switch, %ignored_err, $RA);
 BEGIN {
 	# enforce temporary pool usage for some simple functions
 	my $e;
-	foreach (qw/get_latest_revnum get_uuid get_repos_root/) {
+	foreach (qw/rev_proplist get_latest_revnum get_uuid get_repos_root/) {
 		$e .= "sub $_ {
 			my \$self = shift;
 			my \$pool = SVN::Pool->new;

@@ -2864,29 +2864,7 @@ BEGIN {
 			wantarray ? \@ret : \$ret[0]; }\n";
 	}
 
-	# get_dir needs $pool held in cache for dirents to work,
-	# check_path is cacheable and rev_proplist is close enough
-	# for our purposes.
-	foreach (qw/check_path get_dir rev_proplist/) {
-		$e .= "my \%${_}_cache; my \$${_}_rev = 0; sub $_ {
-			my \$self = shift;
-			my \$r = pop;
-			my \$k = join(\"\\0\", \@_);
-			if (my \$x = \$${_}_cache{\$r}->{\$k}) {
-				return wantarray ? \@\$x : \$x->[0];
-			}
-			my \$pool = SVN::Pool->new;
-			my \@ret = \$self->SUPER::$_(\@_, \$r, \$pool);
-			if (\$r != \$${_}_rev) {
-				\%${_}_cache = ( pool => [] );
-				\$${_}_rev = \$r;
-			}
-			\$${_}_cache{\$r}->{\$k} = \\\@ret;
-			push \@{\$${_}_cache{pool}}, \$pool;
-			wantarray ? \@ret : \$ret[0]; }\n";
-	}
-	$e .= "\n1;";
-	eval $e or die $@;
+	eval "$e; 1;" or die $@;
 }
 
 sub new {

@@ -2919,9 +2897,47 @@ sub new {
 	$self->{svn_path} = $url;
 	$self->{repos_root} = $self->get_repos_root;
 	$self->{svn_path} =~ s#^\Q$self->{repos_root}\E(/|$)##;
+	$self->{cache} = { check_path => { r => 0, data => {} },
+	                   get_dir => { r => 0, data => {} } };
 	$RA = bless $self, $class;
 }
 
+sub check_path {
+	my ($self, $path, $r) = @_;
+	my $cache = $self->{cache}->{check_path};
+	if ($r == $cache->{r} && exists $cache->{data}->{$path}) {
+		return $cache->{data}->{$path};
+	}
+	my $pool = SVN::Pool->new;
+	my $t = $self->SUPER::check_path($path, $r, $pool);
+	$pool->clear;
+	if ($r != $cache->{r}) {
+		%{$cache->{data}} = ();
+		$cache->{r} = $r;
+	}
+	$cache->{data}->{$path} = $t;
+}
+
+sub get_dir {
+	my ($self, $dir, $r) = @_;
+	my $cache = $self->{cache}->{get_dir};
+	if ($r == $cache->{r}) {
+		if (my $x = $cache->{data}->{$dir}) {
+			return wantarray ? @$x : $x->[0];
+		}
+	}
+	my $pool = SVN::Pool->new;
+	my ($d, undef, $props) = $self->SUPER::get_dir($dir, $r, $pool);
+	my %dirents = map { $_ => { kind => $d->{$_}->kind } } keys %$d;
+	$pool->clear;
+	if ($r != $cache->{r}) {
+		%{$cache->{data}} = ();
+		$cache->{r} = $r;
+	}
+	$cache->{data}->{$dir} = [ \%dirents, $r, $props ];
+	wantarray ? (\%dirents, $r, $props) : \%dirents;
+}
+
 sub DESTROY {
 	# do not call the real DESTROY since we store ourselves in $RA
 }

@@ -3136,7 +3152,7 @@ sub match_globs {
 		return unless scalar @x == 3;
 		my $dirents = $x[0];
 		foreach my $de (keys %$dirents) {
-			next if $dirents->{$de}->kind != $SVN::Node::dir;
+			next if $dirents->{$de}->{kind} != $SVN::Node::dir;
 			my $p = $g->{path}->full_path($de);
 			next if $exists->{$p};
 			next if (length $g->{path}->{right} &&

Eric Wong

FYI, with this patch, it actually dies more often. The stack seems to be corrupted so I can't give you a useful stack trace, but here's what gdb says:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000024 0x90002fe0 in strcmp ()
(gdb) where
#0 0x90002fe0 in strcmp ()
#1 0x004e2ac6 in get_private ()
Previous frame inner to this frame (corrupt stack?)

-Steve

Yikes.

Thanks for the feedback. Sorry, I've still been busy with another big project and haven't been able to track down this nasty bug.

If somebody wants to take a stab at working around it: hack up a patch that forks a new child (reusing the main connection) for every revision it needs to fetch. It should still be as fast as normal on *nix machines because fork() is fast and it won't have to renegotiate the connection in every child, but memory leaks and corruption won't have a chance to accumulate.

Eric Wong

This disables some pool allocations, forcing it to use the global one.

Other than that, my eyes are barely open and my brain is having trouble functioning at this time of morning.

index ac44f60..cff81f7 100755
@@ -2883,7 +2883,7 @@ BEGIN {
 			wantarray ? \@ret : \$ret[0]; }\n";
 	}
 	$e .= "\n1;";
-	eval $e or die $@;
+	# eval $e or die $@;
 }
 
 sub new {

@@ -2925,10 +2925,10 @@ sub DESTROY {
 
 sub get_log {
 	my ($self, @args) = @_;
-	my $pool = SVN::Pool->new;
+	# my $pool = SVN::Pool->new;
 	splice(@args, 3, 1) if ($SVN::Core::VERSION le '1.2.0');
-	my $ret = $self->SUPER::get_log(@args, $pool);
-	$pool->clear;
+	my $ret = $self->SUPER::get_log(@args);
+	# $pool->clear;
 	$ret;
 }
 

Eric Wong

The verdict is still out on all patches and attempts I've posted to fix the segfault. I'm quite busy at the moment and won't be attending to git-svn things for another few days.

Eric Wong

Ok, those didn't work. I'm still preoccupied with another project that has a higher priority; and I've come down with a cold so if anybody else wants to step up to the plate, please take a stab at it.

This issue has NOT been known to cause data corruption from any users that experienced it[1]. It seems to be happening in between when changesets are actually fetched.

[1] - if there is any corruption to a particular file, it'll trigger an MD5 mismatch the next time that file is updated.

Eric Wong

I'm getting segfaults in git-svn, too.

This command segfaults reproducably in r55 while running

	git svn clone svn://svn.berlios.de/vermont vermont \
		-T trunk -b branches -t tags

I recompiled the debian subversion 1.4.3dfsg1-1 with debug support, to get a better idea what went wrong.

Relevant output from running this beast under valgrind:
(full valgrind output available at [1])

[...]
r54 = f80e8d0ad785cae5a4abfb1fdd04b31167553ab0 (trunk)
D collector/src/testCollector.h
M collector/src/rcvIpfix.h
M collector/src/rcvMessage.c
M collector/src/common.c
M collector/src/rcvIpfix.c
M collector/src/common.h
r55 = 3fddc1de138e9457385ff43111bcaf6f82884525 (trunk) ==10403==
==10403== Invalid read of size 1

==10403==    at 0x59F90A6: readbuf_getchar (marshal.c:285)
==10403==    by 0x59F90E6: readbuf_getchar_skip_whitespace (marshal.c:294)
==10403==    by 0x59FA0F3: svn_ra_svn_read_item (marshal.c:621)
==10403==    by 0x59FA628: svn_ra_svn_read_tuple (marshal.c:741)
==10403==    by 0x59FA906: svn_ra_svn_read_cmd_response (marshal.c:799)
==10403==    by 0x59ED81D: handle_auth_request (client.c:346)
==10403==    by 0x59F02B1: ra_svn_check_path (client.c:1284)
==10403==    by 0x59B7FD7: svn_ra_check_path (ra_loader.c:564)
==10403==    by 0x599DC93: _wrap_svn_ra_check_path (svn_ra.c:6019)
==10403==    by 0x80BDAD0: Perl_pp_entersub (in /usr/bin/perl)
==10403==    by 0x80BC3A8: Perl_runops_standard (in /usr/bin/perl)
==10403==    by 0x8063A1A: perl_run (in /usr/bin/perl)

==10403== Address 0x6F6C6720 is not stack'd, malloc'd or (recently) free'd ==10403==
==10403== Process terminating with default action of signal 11 (SIGSEGV) ==10403== Access not within mapped region at address 0x6F6C6720

==10403==    at 0x59F90A6: readbuf_getchar (marshal.c:285)
==10403==    by 0x59F90E6: readbuf_getchar_skip_whitespace (marshal.c:294)
==10403==    by 0x59FA0F3: svn_ra_svn_read_item (marshal.c:621)
==10403==    by 0x59FA628: svn_ra_svn_read_tuple (marshal.c:741)
==10403==    by 0x59FA906: svn_ra_svn_read_cmd_response (marshal.c:799)
==10403==    by 0x59ED81D: handle_auth_request (client.c:346)
==10403==    by 0x59F02B1: ra_svn_check_path (client.c:1284)
==10403==    by 0x59B7FD7: svn_ra_check_path (ra_loader.c:564)
==10403==    by 0x599DC93: _wrap_svn_ra_check_path (svn_ra.c:6019)
==10403==    by 0x80BDAD0: Perl_pp_entersub (in /usr/bin/perl)
==10403==    by 0x80BC3A8: Perl_runops_standard (in /usr/bin/perl)
==10403==    by 0x8063A1A: perl_run (in /usr/bin/perl)

==10403==
==10403== ERROR SUMMARY: 199 errors from 26 contexts (suppressed: 19 from 1) ==10403== malloc/free: in use at exit: 7,028,898 bytes in 129,440 blocks. ==10403== malloc/free: 325,184 allocs, 195,744 frees, 21,303,514 bytes allocated. ==10403== For counts of detected errors, rerun with: -v
==10403== searching for pointers to 129,440 not-freed blocks. ==10403== checked 7,214,984 bytes.
==10403==
==10403== LEAK SUMMARY:
==10403== definitely lost: 4,657 bytes in 118 blocks.
==10403== possibly lost: 216 bytes in 13 blocks.
==10403== still reachable: 7,024,025 bytes in 129,309 blocks.
==10403== suppressed: 0 bytes in 0 blocks.
==10403== Use --leak-check=full to see details of leaked memory.

===========================================================================

subversion/libsvn_ra_svn/marshal.c:
[...]
280 static svn_error_t *readbuf_getchar(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
281 char *result)
282 {
283 if (conn->read_ptr == conn->read_end)
284 SVN_ERR(readbuf_fill(conn, pool));
285 *result = *conn->read_ptr++; <-------- SEGFAULT happens here
286 return SVN_NO_ERROR;
287 }

===========================================================================

gdb backtrace:

r54 = f80e8d0ad785cae5a4abfb1fdd04b31167553ab0 (trunk)

        D       collector/src/testCollector.h
        M       collector/src/rcvIpfix.h
        M       collector/src/rcvMessage.c
        M       collector/src/common.c
        M       collector/src/rcvIpfix.c
        M       collector/src/common.h

r55 = 3fddc1de138e9457385ff43111bcaf6f82884525 (trunk)

Program received signal SIGSEGV, Segmentation fault. [Switching to Thread -1210452288 (LWP 17426)] 0xb745f0a6 in readbuf_getchar (conn=0x877bf78, pool=0x88a6178,

    result=0xbfea011f "\bL:F�L:F�X\001�\006F�x�w\bxa\212\bD\001�230��x�w\b�\212\b�F�\002")
    at /tmp/buildd/subversion-1.4.3dfsg1/subversion/libsvn_ra_svn/marshal.c:285
285       *result = *conn->read_ptr++;
(gdb) bt
#0  0xb745f0a6 in readbuf_getchar (conn=0x877bf78, pool=0x88a6178, 
    result=0xbfea011f "\bL:F�L:F�X\001�\006F�x�w\bxa\212\bD\001�230��x�w\b�\212\b�F�\002")
    at /tmp/buildd/subversion-1.4.3dfsg1/subversion/libsvn_ra_svn/marshal.c:285
#1  0xb745f0e7 in readbuf_getchar_skip_whitespace (conn=0x877bf78, pool=0x88a6178, 
    result=0xbfea011f "\bL:F�L:F�X\001�\006F�x�w\bxa\212\bD\001�230��x�w\b�\212\b�F�\002")
    at /tmp/buildd/subversion-1.4.3dfsg1/subversion/libsvn_ra_svn/marshal.c:294
#2  0xb74600f4 in svn_ra_svn_read_item (conn=0x877bf78, pool=0x88a6178, item=0xbfea0144)
    at /tmp/buildd/subversion-1.4.3dfsg1/subversion/libsvn_ra_svn/marshal.c:621
#3  0xb7460629 in svn_ra_svn_read_tuple (conn=0x877bf78, pool=0x88a6178, fmt=0xb7462607 "wl")
    at /tmp/buildd/subversion-1.4.3dfsg1/subversion/libsvn_ra_svn/marshal.c:741
#4  0xb7460907 in svn_ra_svn_read_cmd_response (conn=0x877bf78, pool=0x88a6178, fmt=0xb7461372 "lc")
    at /tmp/buildd/subversion-1.4.3dfsg1/subversion/libsvn_ra_svn/marshal.c:799
#5  0xb745381e in handle_auth_request (sess=0x877e1e8, pool=0x88a6178)
    at /tmp/buildd/subversion-1.4.3dfsg1/subversion/libsvn_ra_svn/client.c:346
#6  0xb74562b2 in ra_svn_check_path (session=0x8778090, path=0x8899d48 "tags/collector-orig", rev=56, kind=0xbfea02c8, pool=0x88a6178)
    at /tmp/buildd/subversion-1.4.3dfsg1/subversion/libsvn_ra_svn/client.c:1284
#7  0xb7495fd8 in svn_ra_check_path (session=0x8778090, path=0x8899d48 "tags/collector-orig", revision=56, kind=0xbfea02c8, 
    pool=0x88a6178) at /tmp/buildd/subversion-1.4.3dfsg1/subversion/libsvn_ra/ra_loader.c:564

#8 0xb74c2c94 in _wrap_svn_ra_check_path (my_perl=0x814e008, cv=0x85e1a94) at svn_ra.c:6019 #9 0x080bdad1 in Perl_pp_entersub ()
#10 0x080bc3a9 in Perl_runops_standard ()
#11 0x08063a1b in perl_run ()
#12 0x0805ffd1 in main ()
(gdb)

I'm a little lost in debuging this any further. So if anyone could give me some advice ... feel free to contact me by mail or in #git (nickname siprbaum)

-Peter

My system is debian etch with backported subversion + patch in [2] and git from debian unstable/experimental

ii  git-core                        1.5.1.3-1
ii  git-svn                         1.5.1.3-1
ii  libsvn-perl                     1.4.3dfsg1-1
ii  libsvn1                         1.4.3dfsg1-1
ii  subversion                      1.4.3dfsg1-1

[1]: http://wwwcip.informatik.uni-erlangen.de/~siprbaum/git-svn-segfault.txt
[2]: http://svn.haxx.se/dev/archive-2007-01/0936.shtml

Both repositories seem to be cloning fine after the patch here:

http://permalink.gmane.org/gmane.comp.version-control.git/47128

I really don't know what's going on with libsvn, either :(

Eric Wong

I can confirm that the patch fixes the segfault for me too. Thanks!

-Steve

I can (hopefully) confirm this, too.

-Peter

Side Note:

I tried it yesterday and got a message which
looks something alike "connection closed ..." and after continuing the import I got an segfault. But as I couldn't reproduce this today, it seems that it was just a flacky connection and after the closed connection the repo was just in an inconsistent state.

I'll propably try to import this repo a few more times, just to check if it is really fixed.