t9128 failing randomly with svn 1.9?

t9128 failing randomly with svn 1.9?

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

While running tests for my object_id part 11 series, I noticed a random
segfault in git svn while running t9128.  I've reproduced this on a
different machine as well, using both Subversion 1.9.5 and 1.9.7 (Debian
stable and unstable).  It is reproducible on master.

When the test fails, it fails on the "git svn tag tag3" step, and I get
the following:

Copying file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/trunk at r2 to file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/tags/tag3...
Found possible branch point: file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/trunk => file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/tags/tag3, 2
Found branch parent: (refs/remotes/origin/tags/tag3) 0604824a81a121ad05aaf8caea65d8ca8f86c018
Following parent with do_switch
Successfully followed parent
r7 = f8467f2cee3bcead03e84cb51cf44f467a87457d (refs/remotes/origin/tags/tag3)
error: git-svn died of signal 11

Doing the following three times, I had two crashes.

(set -e; for i in $(seq 1 20); do (cd t && ./t9128-git-svn-cmd-branch.sh --verbose); done)

I'm not really familiar with git svn or its internals, and I didn't see
anything recently on the list about this.  Is this issue known?
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

Re: t9128 failing randomly with svn 1.9?

[Todd Zullinger]

[-- Attachment #1: Type: text/plain, Size: 2494 bytes --]

brian m. carlson wrote:
> While running tests for my object_id part 11 series, I noticed a random
> segfault in git svn while running t9128.  I've reproduced this on a
> different machine as well, using both Subversion 1.9.5 and 1.9.7 (Debian
> stable and unstable).  It is reproducible on master.
> 
> When the test fails, it fails on the "git svn tag tag3" step, and I get
> the following:
> 
> Copying file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/trunk at r2 to file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/tags/tag3...
> Found possible branch point: file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/trunk => file:///home/bmc/checkouts/git/t/trash%20directory.t9128-git-svn-cmd-branch/svnrepo/tags/tag3, 2
> Found branch parent: (refs/remotes/origin/tags/tag3) 0604824a81a121ad05aaf8caea65d8ca8f86c018
> Following parent with do_switch
> Successfully followed parent
> r7 = f8467f2cee3bcead03e84cb51cf44f467a87457d (refs/remotes/origin/tags/tag3)
> error: git-svn died of signal 11
> 
> Doing the following three times, I had two crashes.
> 
> (set -e; for i in $(seq 1 20); do (cd t && ./t9128-git-svn-cmd-branch.sh --verbose); done)
> 
> I'm not really familiar with git svn or its internals, and I didn't see
> anything recently on the list about this.  Is this issue known?

I see the same test fail randomly on Fedora (and I think on
CentOS/RHEL, but I run the full tests more often on Fedora).

For me, it's tests 3 and 4 which fail with the same error.
I thought it was a failure in subversion or the perl
bindings rather than git, so I simply disabled them in the
Fedora builds.  The Debian packages skip 9128 as well (and
9167, which fails similarly).

I've seen the failures in t9141 from 'git svn branch' as
well.  I made a note to re-enable those tests after Jeff's
work to make it easier to run with shell tracing enabled by
default, but have not done so yet.

The 'git svn' tests are not run in Travis because the perl
subversion bindings are not installed.  I haven't made time
to try installing them and running the tests in Travis to
see if the failures occur there, but I suspect they would.

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Going to trial with a lawyer who considers your whole life-style a
Crime in Progress is not a happy prospect.
    -- Hunter S. Thompson


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 543 bytes --]

Re: t9128 failing randomly with svn 1.9?

[Eric Wong]

Todd Zullinger <tmz@pobox.com> wrote:
> brian m. carlson wrote:
> > r7 = f8467f2cee3bcead03e84cb51cf44f467a87457d (refs/remotes/origin/tags/tag3)
> > error: git-svn died of signal 11
> > 
> > Doing the following three times, I had two crashes.
> > 
> > (set -e; for i in $(seq 1 20); do (cd t && ./t9128-git-svn-cmd-branch.sh --verbose); done)
> > 
> > I'm not really familiar with git svn or its internals, and I didn't see
> > anything recently on the list about this.  Is this issue known?

The SVN bindings have been known to crash occasionally and I
can't really get my head around XS/Swig or Perl internals :/
Looking at the coredump, it seems to be around exit and
destruction (only Perl in the stacktrace, nothing from libsvn).
So I suspect it's a lifetime or ordering bug, testing a patch
below (slowly).

"branch" is also the only place we use SVN::Client->new these
days.

> For me, it's tests 3 and 4 which fail with the same error.
> I thought it was a failure in subversion or the perl
> bindings rather than git, so I simply disabled them in the
> Fedora builds.  The Debian packages skip 9128 as well (and
> 9167, which fails similarly).
> 
> I've seen the failures in t9141 from 'git svn branch' as
> well.  I made a note to re-enable those tests after Jeff's
> work to make it easier to run with shell tracing enabled by
> default, but have not done so yet.

Just a guess, but it might be related to destruction order.
Running t9128 on a 32-bit Pentium-M, it took me 39 tries to
fail.

diff --git a/git-svn.perl b/git-svn.perl
index 76a75d0b3d..2ba14269bb 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1200,6 +1200,11 @@ sub cmd_branch {
 	$ctx->copy($src, $rev, $dst)
 		unless $_dry_run;
 
+	# Release resources held by ctx before creating another SVN::Ra
+	# so destruction is orderly.  This seems necessary Subversion 1.9.5
+	# to avoid segfaults.
+	$ctx = undef;
+
 	$gs->fetch_all;
 }
 

I'll be looping t9128, t9141 and t9167 with that for a few
hours or day.  Will report back sooner if it fails.
I'm on an ancient 32-bit system, I guess you guys encountered
it on 64-bit machines?

Re: t9128 failing randomly with svn 1.9?

[Todd Zullinger]

Eric Wong wrote:
> Todd Zullinger <tmz@pobox.com> wrote:
> Just a guess, but it might be related to destruction order.
> Running t9128 on a 32-bit Pentium-M, it took me 39 tries to
> fail.
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 76a75d0b3d..2ba14269bb 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1200,6 +1200,11 @@ sub cmd_branch {
>  	$ctx->copy($src, $rev, $dst)
>  		unless $_dry_run;
>  
> +	# Release resources held by ctx before creating another SVN::Ra
> +	# so destruction is orderly.  This seems necessary Subversion 1.9.5
> +	# to avoid segfaults.
> +	$ctx = undef;
> +
>  	$gs->fetch_all;
>  }
>  
> I'll be looping t9128, t9141 and t9167 with that for a few
> hours or day.  Will report back sooner if it fails.
> I'm on an ancient 32-bit system, I guess you guys encountered
> it on 64-bit machines?

Yeah.  I saw it on numerous architectures, x86 and x86_64.
I believe I saw it on aarch64 and ppc as well, but I don't
have build logs at hand to confirm.

I'm running the tests with and without your patch as well.
So far I've run t9128 300 times with the patch and no
failures.  Without it, it's failed 3 times in only a few
dozen runs.  That's promising.

Thanks for poking this Eric, and Brian for bringing it up.
I had intended to look at it again and mention it,
eventually. :)

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I figure that if God actually does exist, He's big enough to
understand an honest difference of opinion.
    -- Isaac Asimov

[PATCH] git-svn: control destruction order to avoid segfault

[Eric Wong]

Todd Zullinger <tmz@pobox.com> wrote:
> I'm running the tests with and without your patch as well.
> So far I've run t9128 300 times with the patch and no
> failures.  Without it, it's failed 3 times in only a few
> dozen runs.  That's promising.

Thanks for confirming it works on other systems.
Pull request and patch below:

The following changes since commit 5be1f00a9a701532232f57958efab4be8c959a29:

  First batch after 2.16 (2018-01-23 13:21:10 -0800)

are available in the Git repository at:

  git://bogomips.org/git-svn.git svn-branch-segfault

for you to fetch changes up to 2784b8d68faca823489949cbc69ead2f296cfc07:

  git-svn: control destruction order to avoid segfault (2018-01-29 23:12:00 +0000)

----------------------------------------------------------------
Eric Wong (1):
      git-svn: control destruction order to avoid segfault

 git-svn.perl | 5 +++++
 1 file changed, 5 insertions(+)

---------8<---------
Subject: [PATCH] git-svn: control destruction order to avoid segfault

It seems necessary to control destruction ordering to avoid a
segfault with SVN 1.9.5 when using "git svn branch".
I've also reported the problem against libsvn-perl to Debian
[Bug #888791], but releasing the SVN::Client instance can be
beneficial anyways to save memory.

ref: https://bugs.debian.org/888791
Tested-by: Todd Zullinger <tmz@pobox.com>
Reported-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Eric Wong <e@80x24.org>
---
 git-svn.perl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/git-svn.perl b/git-svn.perl
index 76a75d0b3d..a6b6c3e40c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1200,6 +1200,11 @@ sub cmd_branch {
 	$ctx->copy($src, $rev, $dst)
 		unless $_dry_run;
 
+	# Release resources held by ctx before creating another SVN::Ra
+	# so destruction is orderly.  This seems necessary with SVN 1.9.5
+	# to avoid segfaults.
+	$ctx = undef;
+
 	$gs->fetch_all;
 }
 
-- 
EW

Re: [PATCH] git-svn: control destruction order to avoid segfault

[Junio C Hamano]

Eric Wong <e@80x24.org> writes:

> Todd Zullinger <tmz@pobox.com> wrote:
>> I'm running the tests with and without your patch as well.
>> So far I've run t9128 300 times with the patch and no
>> failures.  Without it, it's failed 3 times in only a few
>> dozen runs.  That's promising.
>
> Thanks for confirming it works on other systems.
> Pull request and patch below:
>
> The following changes since commit 5be1f00a9a701532232f57958efab4be8c959a29:
>
>   First batch after 2.16 (2018-01-23 13:21:10 -0800)
>
> are available in the Git repository at:
>
>   git://bogomips.org/git-svn.git svn-branch-segfault
>
> for you to fetch changes up to 2784b8d68faca823489949cbc69ead2f296cfc07:
>
>   git-svn: control destruction order to avoid segfault (2018-01-29 23:12:00 +0000)
>
> ----------------------------------------------------------------
> Eric Wong (1):
>       git-svn: control destruction order to avoid segfault
>
>  git-svn.perl | 5 +++++
>  1 file changed, 5 insertions(+)

Thanks.  I'd actually apply this as a patch instead of pullilng, as
I suspect you'd want it in 'maint' as well, though.


> ---------8<---------
> Subject: [PATCH] git-svn: control destruction order to avoid segfault
>
> It seems necessary to control destruction ordering to avoid a
> segfault with SVN 1.9.5 when using "git svn branch".
> I've also reported the problem against libsvn-perl to Debian
> [Bug #888791], but releasing the SVN::Client instance can be
> beneficial anyways to save memory.
>
> ref: https://bugs.debian.org/888791
> Tested-by: Todd Zullinger <tmz@pobox.com>
> Reported-by: brian m. carlson <sandals@crustytoothpaste.net>
> Signed-off-by: Eric Wong <e@80x24.org>
> ---
>  git-svn.perl | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 76a75d0b3d..a6b6c3e40c 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1200,6 +1200,11 @@ sub cmd_branch {
>  	$ctx->copy($src, $rev, $dst)
>  		unless $_dry_run;
>  
> +	# Release resources held by ctx before creating another SVN::Ra
> +	# so destruction is orderly.  This seems necessary with SVN 1.9.5
> +	# to avoid segfaults.
> +	$ctx = undef;
> +
>  	$gs->fetch_all;
>  }

Re: t9128 failing randomly with svn 1.9?

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]

On Mon, Jan 29, 2018 at 12:06:27PM +0000, Eric Wong wrote:
> Todd Zullinger <tmz@pobox.com> wrote:
> diff --git a/git-svn.perl b/git-svn.perl
> index 76a75d0b3d..2ba14269bb 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1200,6 +1200,11 @@ sub cmd_branch {
>  	$ctx->copy($src, $rev, $dst)
>  		unless $_dry_run;
>  
> +	# Release resources held by ctx before creating another SVN::Ra
> +	# so destruction is orderly.  This seems necessary Subversion 1.9.5
> +	# to avoid segfaults.
> +	$ctx = undef;
> +

This may be the right thing to do.  I've seen a decent number of cases
in Perl where global destruction randomly causes segfaults.

>  	$gs->fetch_all;
>  }
>  
> 
> I'll be looping t9128, t9141 and t9167 with that for a few
> hours or day.  Will report back sooner if it fails.
> I'm on an ancient 32-bit system, I guess you guys encountered
> it on 64-bit machines?

Yes, both systems are 64-bit Debian systems, one stable, and the other
unstable.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

Re: t9128 failing randomly with svn 1.9?

[Todd Zullinger]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

I wrote:
> The 'git svn' tests are not run in Travis because the perl
> subversion bindings are not installed.  I haven't made time
> to try installing them and running the tests in Travis to
> see if the failures occur there, but I suspect they would.

Before anyone spends time wondering about this, I was
apparently looking at the MacOS build logs rather than the
Linux logs.  The git svn tests are indeed run in Travis, as
long as you look at the right build logs. :/

Sadly (or amusingly), I think I looked at this before and
made the same mistake, realized it, and then did it again
today.  Hopefully if I make the same mistake again I'll
realize it before I post it to the list. ;)

Anyway, the Travis Linux container uses Ubuntu Trusty, which
has subversion 1.8.8.  I suppose that's why these random
failures haven't turned up there.

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I got stopped by a cop the other day.  He said, "Why'd you run that
stop sign?"  I said, "Because I don't believe everything I read."
    -- Stephen Wright


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 543 bytes --]