{"thread":{"id":"47709","subject":"t9128 failing randomly with svn 1.9?","startedAt":"2018-01-29T01:51:49Z","lastAt":"2018-01-30T21:15:28Z","messageCount":8,"participants":["brian m. carlson","Todd Zullinger","Eric Wong","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"337647","messageId":"20180129015134.GN431130@genre.crustytoothpaste.net","threadId":"47709","inReplyTo":null,"subject":"t9128 failing randomly with svn 1.9?","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-29T01:51:34Z","receivedAt":"2018-01-29T01:51:49Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"While running tests for my object_id part 11 series, I noticed a random\nsegfault in git svn while running t9128.  I've reproduced this on a\ndifferent machine as well, using both Subversion 1.9.5 and 1.9.7 (Debian\nstable and unstable).  It is reproducible on master.\n\nWhen the test fails, it fails on the \"git svn tag tag3\" step, and I get\nthe following:\n\nCopying 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...\nFound 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\nFound branch parent: (refs/remotes/origin/tags/tag3) 0604824a81a121ad05aaf8caea65d8ca8f86c018\nFollowing parent with do_switch\nSuccessfully followed parent\nr7 = f8467f2cee3bcead03e84cb51cf44f467a87457d (refs/remotes/origin/tags/tag3)\nerror: git-svn died of signal 11\n\nDoing the following three times, I had two crashes.\n\n(set -e; for i in $(seq 1 20); do (cd t && ./t9128-git-svn-cmd-branch.sh --verbose); done)\n\nI'm not really familiar with git svn or its internals, and I didn't see\nanything recently on the list about this.  Is this issue known?\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"337648","messageId":"20180129025812.GD1427@zaya.teonanacatl.net","threadId":"47709","inReplyTo":"20180129015134.GN431130@genre.crustytoothpaste.net","subject":"Re: t9128 failing randomly with svn 1.9?","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-01-29T02:58:12Z","receivedAt":"2018-01-29T02:58:20Z","isPatch":false,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"brian m. carlson wrote:\n> While running tests for my object_id part 11 series, I noticed a random\n> segfault in git svn while running t9128.  I've reproduced this on a\n> different machine as well, using both Subversion 1.9.5 and 1.9.7 (Debian\n> stable and unstable).  It is reproducible on master.\n> \n> When the test fails, it fails on the \"git svn tag tag3\" step, and I get\n> the following:\n> \n> 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...\n> 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\n> Found branch parent: (refs/remotes/origin/tags/tag3) 0604824a81a121ad05aaf8caea65d8ca8f86c018\n> Following parent with do_switch\n> Successfully followed parent\n> r7 = f8467f2cee3bcead03e84cb51cf44f467a87457d (refs/remotes/origin/tags/tag3)\n> error: git-svn died of signal 11\n> \n> Doing the following three times, I had two crashes.\n> \n> (set -e; for i in $(seq 1 20); do (cd t && ./t9128-git-svn-cmd-branch.sh --verbose); done)\n> \n> I'm not really familiar with git svn or its internals, and I didn't see\n> anything recently on the list about this.  Is this issue known?\n\nI see the same test fail randomly on Fedora (and I think on\nCentOS/RHEL, but I run the full tests more often on Fedora).\n\nFor me, it's tests 3 and 4 which fail with the same error.\nI thought it was a failure in subversion or the perl\nbindings rather than git, so I simply disabled them in the\nFedora builds.  The Debian packages skip 9128 as well (and\n9167, which fails similarly).\n\nI've seen the failures in t9141 from 'git svn branch' as\nwell.  I made a note to re-enable those tests after Jeff's\nwork to make it easier to run with shell tracing enabled by\ndefault, but have not done so yet.\n\nThe 'git svn' tests are not run in Travis because the perl\nsubversion bindings are not installed.  I haven't made time\nto try installing them and running the tests in Travis to\nsee if the failures occur there, but I suspect they would.\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nGoing to trial with a lawyer who considers your whole life-style a\nCrime in Progress is not a happy prospect.\n    -- Hunter S. Thompson\n\n"},{"id":"337657","messageId":"20180129120627.al2xvx4yhhvwn6ih@untitled","threadId":"47709","inReplyTo":"20180129025812.GD1427@zaya.teonanacatl.net","subject":"Re: t9128 failing randomly with svn 1.9?","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2018-01-29T12:06:27Z","receivedAt":"2018-01-29T12:06:32Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Todd Zullinger <tmz@pobox.com> wrote:\n> brian m. carlson wrote:\n> > r7 = f8467f2cee3bcead03e84cb51cf44f467a87457d (refs/remotes/origin/tags/tag3)\n> > error: git-svn died of signal 11\n> > \n> > Doing the following three times, I had two crashes.\n> > \n> > (set -e; for i in $(seq 1 20); do (cd t && ./t9128-git-svn-cmd-branch.sh --verbose); done)\n> > \n> > I'm not really familiar with git svn or its internals, and I didn't see\n> > anything recently on the list about this.  Is this issue known?\n\nThe SVN bindings have been known to crash occasionally and I\ncan't really get my head around XS/Swig or Perl internals :/\nLooking at the coredump, it seems to be around exit and\ndestruction (only Perl in the stacktrace, nothing from libsvn).\nSo I suspect it's a lifetime or ordering bug, testing a patch\nbelow (slowly).\n\n\"branch\" is also the only place we use SVN::Client->new these\ndays.\n\n> For me, it's tests 3 and 4 which fail with the same error.\n> I thought it was a failure in subversion or the perl\n> bindings rather than git, so I simply disabled them in the\n> Fedora builds.  The Debian packages skip 9128 as well (and\n> 9167, which fails similarly).\n> \n> I've seen the failures in t9141 from 'git svn branch' as\n> well.  I made a note to re-enable those tests after Jeff's\n> work to make it easier to run with shell tracing enabled by\n> default, but have not done so yet.\n\nJust a guess, but it might be related to destruction order.\nRunning t9128 on a 32-bit Pentium-M, it took me 39 tries to\nfail.\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 76a75d0b3d..2ba14269bb 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -1200,6 +1200,11 @@ sub cmd_branch {\n \t$ctx->copy($src, $rev, $dst)\n \t\tunless $_dry_run;\n \n+\t# Release resources held by ctx before creating another SVN::Ra\n+\t# so destruction is orderly.  This seems necessary Subversion 1.9.5\n+\t# to avoid segfaults.\n+\t$ctx = undef;\n+\n \t$gs->fetch_all;\n }\n \n\nI'll be looping t9128, t9141 and t9167 with that for a few\nhours or day.  Will report back sooner if it fails.\nI'm on an ancient 32-bit system, I guess you guys encountered\nit on 64-bit machines?\n"},{"id":"337679","messageId":"20180129184345.GI1427@zaya.teonanacatl.net","threadId":"47709","inReplyTo":"20180129120627.al2xvx4yhhvwn6ih@untitled","subject":"Re: t9128 failing randomly with svn 1.9?","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-01-29T18:43:45Z","receivedAt":"2018-01-29T18:43:58Z","isPatch":false,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Eric Wong wrote:\n> Todd Zullinger <tmz@pobox.com> wrote:\n> Just a guess, but it might be related to destruction order.\n> Running t9128 on a 32-bit Pentium-M, it took me 39 tries to\n> fail.\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index 76a75d0b3d..2ba14269bb 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -1200,6 +1200,11 @@ sub cmd_branch {\n>  \t$ctx->copy($src, $rev, $dst)\n>  \t\tunless $_dry_run;\n>  \n> +\t# Release resources held by ctx before creating another SVN::Ra\n> +\t# so destruction is orderly.  This seems necessary Subversion 1.9.5\n> +\t# to avoid segfaults.\n> +\t$ctx = undef;\n> +\n>  \t$gs->fetch_all;\n>  }\n>  \n> I'll be looping t9128, t9141 and t9167 with that for a few\n> hours or day.  Will report back sooner if it fails.\n> I'm on an ancient 32-bit system, I guess you guys encountered\n> it on 64-bit machines?\n\nYeah.  I saw it on numerous architectures, x86 and x86_64.\nI believe I saw it on aarch64 and ppc as well, but I don't\nhave build logs at hand to confirm.\n\nI'm running the tests with and without your patch as well.\nSo far I've run t9128 300 times with the patch and no\nfailures.  Without it, it's failed 3 times in only a few\ndozen runs.  That's promising.\n\nThanks for poking this Eric, and Brian for bringing it up.\nI had intended to look at it again and mention it,\neventually. :)\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nI figure that if God actually does exist, He's big enough to\nunderstand an honest difference of opinion.\n    -- Isaac Asimov\n\n"},{"id":"337765","messageId":"20180129231653.GA22834@starla","threadId":"47709","inReplyTo":"20180129184345.GI1427@zaya.teonanacatl.net","subject":"[PATCH] git-svn: control destruction order to avoid segfault","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2018-01-29T23:16:53Z","receivedAt":"2018-01-29T23:16:58Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Todd Zullinger <tmz@pobox.com> wrote:\n> I'm running the tests with and without your patch as well.\n> So far I've run t9128 300 times with the patch and no\n> failures.  Without it, it's failed 3 times in only a few\n> dozen runs.  That's promising.\n\nThanks for confirming it works on other systems.\nPull request and patch below:\n\nThe following changes since commit 5be1f00a9a701532232f57958efab4be8c959a29:\n\n  First batch after 2.16 (2018-01-23 13:21:10 -0800)\n\nare available in the Git repository at:\n\n  git://bogomips.org/git-svn.git svn-branch-segfault\n\nfor you to fetch changes up to 2784b8d68faca823489949cbc69ead2f296cfc07:\n\n  git-svn: control destruction order to avoid segfault (2018-01-29 23:12:00 +0000)\n\n----------------------------------------------------------------\nEric Wong (1):\n      git-svn: control destruction order to avoid segfault\n\n git-svn.perl | 5 +++++\n 1 file changed, 5 insertions(+)\n\n---------8<---------\nSubject: [PATCH] git-svn: control destruction order to avoid segfault\n\nIt seems necessary to control destruction ordering to avoid a\nsegfault with SVN 1.9.5 when using \"git svn branch\".\nI've also reported the problem against libsvn-perl to Debian\n[Bug #888791], but releasing the SVN::Client instance can be\nbeneficial anyways to save memory.\n\nref: https://bugs.debian.org/888791\nTested-by: Todd Zullinger <tmz@pobox.com>\nReported-by: brian m. carlson <sandals@crustytoothpaste.net>\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n git-svn.perl | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 76a75d0b3d..a6b6c3e40c 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -1200,6 +1200,11 @@ sub cmd_branch {\n \t$ctx->copy($src, $rev, $dst)\n \t\tunless $_dry_run;\n \n+\t# Release resources held by ctx before creating another SVN::Ra\n+\t# so destruction is orderly.  This seems necessary with SVN 1.9.5\n+\t# to avoid segfaults.\n+\t$ctx = undef;\n+\n \t$gs->fetch_all;\n }\n \n-- \nEW\n"},{"id":"337770","messageId":"20180130002539.GO431130@genre.crustytoothpaste.net","threadId":"47709","inReplyTo":"20180129120627.al2xvx4yhhvwn6ih@untitled","subject":"Re: t9128 failing randomly with svn 1.9?","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-01-30T00:25:39Z","receivedAt":"2018-01-30T00:25:52Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Jan 29, 2018 at 12:06:27PM +0000, Eric Wong wrote:\n> Todd Zullinger <tmz@pobox.com> wrote:\n> diff --git a/git-svn.perl b/git-svn.perl\n> index 76a75d0b3d..2ba14269bb 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -1200,6 +1200,11 @@ sub cmd_branch {\n>  \t$ctx->copy($src, $rev, $dst)\n>  \t\tunless $_dry_run;\n>  \n> +\t# Release resources held by ctx before creating another SVN::Ra\n> +\t# so destruction is orderly.  This seems necessary Subversion 1.9.5\n> +\t# to avoid segfaults.\n> +\t$ctx = undef;\n> +\n\nThis may be the right thing to do.  I've seen a decent number of cases\nin Perl where global destruction randomly causes segfaults.\n\n>  \t$gs->fetch_all;\n>  }\n>  \n> \n> I'll be looping t9128, t9141 and t9167 with that for a few\n> hours or day.  Will report back sooner if it fails.\n> I'm on an ancient 32-bit system, I guess you guys encountered\n> it on 64-bit machines?\n\nYes, both systems are 64-bit Debian systems, one stable, and the other\nunstable.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"337773","messageId":"20180130030931.GK1427@zaya.teonanacatl.net","threadId":"47709","inReplyTo":"20180129025812.GD1427@zaya.teonanacatl.net","subject":"Re: t9128 failing randomly with svn 1.9?","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-01-30T03:09:31Z","receivedAt":"2018-01-30T03:09:38Z","isPatch":false,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"I wrote:\n> The 'git svn' tests are not run in Travis because the perl\n> subversion bindings are not installed.  I haven't made time\n> to try installing them and running the tests in Travis to\n> see if the failures occur there, but I suspect they would.\n\nBefore anyone spends time wondering about this, I was\napparently looking at the MacOS build logs rather than the\nLinux logs.  The git svn tests are indeed run in Travis, as\nlong as you look at the right build logs. :/\n\nSadly (or amusingly), I think I looked at this before and\nmade the same mistake, realized it, and then did it again\ntoday.  Hopefully if I make the same mistake again I'll\nrealize it before I post it to the list. ;)\n\nAnyway, the Travis Linux container uses Ubuntu Trusty, which\nhas subversion 1.8.8.  I suppose that's why these random\nfailures haven't turned up there.\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nI got stopped by a cop the other day.  He said, \"Why'd you run that\nstop sign?\"  I said, \"Because I don't believe everything I read.\"\n    -- Stephen Wright\n\n"},{"id":"337815","messageId":"xmqqvafjrqu1.fsf@gitster-ct.c.googlers.com","threadId":"47709","inReplyTo":"20180129231653.GA22834@starla","subject":"Re: [PATCH] git-svn: control destruction order to avoid segfault","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-30T21:15:18Z","receivedAt":"2018-01-30T21:15:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <e@80x24.org> writes:\n\n> Todd Zullinger <tmz@pobox.com> wrote:\n>> I'm running the tests with and without your patch as well.\n>> So far I've run t9128 300 times with the patch and no\n>> failures.  Without it, it's failed 3 times in only a few\n>> dozen runs.  That's promising.\n>\n> Thanks for confirming it works on other systems.\n> Pull request and patch below:\n>\n> The following changes since commit 5be1f00a9a701532232f57958efab4be8c959a29:\n>\n>   First batch after 2.16 (2018-01-23 13:21:10 -0800)\n>\n> are available in the Git repository at:\n>\n>   git://bogomips.org/git-svn.git svn-branch-segfault\n>\n> for you to fetch changes up to 2784b8d68faca823489949cbc69ead2f296cfc07:\n>\n>   git-svn: control destruction order to avoid segfault (2018-01-29 23:12:00 +0000)\n>\n> ----------------------------------------------------------------\n> Eric Wong (1):\n>       git-svn: control destruction order to avoid segfault\n>\n>  git-svn.perl | 5 +++++\n>  1 file changed, 5 insertions(+)\n\nThanks.  I'd actually apply this as a patch instead of pullilng, as\nI suspect you'd want it in 'maint' as well, though.\n\n\n> ---------8<---------\n> Subject: [PATCH] git-svn: control destruction order to avoid segfault\n>\n> It seems necessary to control destruction ordering to avoid a\n> segfault with SVN 1.9.5 when using \"git svn branch\".\n> I've also reported the problem against libsvn-perl to Debian\n> [Bug #888791], but releasing the SVN::Client instance can be\n> beneficial anyways to save memory.\n>\n> ref: https://bugs.debian.org/888791\n> Tested-by: Todd Zullinger <tmz@pobox.com>\n> Reported-by: brian m. carlson <sandals@crustytoothpaste.net>\n> Signed-off-by: Eric Wong <e@80x24.org>\n> ---\n>  git-svn.perl | 5 +++++\n>  1 file changed, 5 insertions(+)\n>\n> diff --git a/git-svn.perl b/git-svn.perl\n> index 76a75d0b3d..a6b6c3e40c 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -1200,6 +1200,11 @@ sub cmd_branch {\n>  \t$ctx->copy($src, $rev, $dst)\n>  \t\tunless $_dry_run;\n>  \n> +\t# Release resources held by ctx before creating another SVN::Ra\n> +\t# so destruction is orderly.  This seems necessary with SVN 1.9.5\n> +\t# to avoid segfaults.\n> +\t$ctx = undef;\n> +\n>  \t$gs->fetch_all;\n>  }\n"}]}