{"thread":{"id":"34838","subject":"[PATCH] git-svn: Fix termination issues for remote svn connections","startedAt":"2013-09-03T07:35:29Z","lastAt":"2013-09-09T15:42:10Z","messageCount":12,"participants":["Uli Heller","Junio C Hamano","Eric Wong","Kyle J. McKay"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"226597","messageId":"6970c0cab40c60195c8f042a3b930a0a.squirrel@83.236.132.106","threadId":"34838","inReplyTo":null,"subject":"[PATCH] git-svn: Fix termination issues for remote svn connections","fromName":"Uli Heller","fromEmail":"uli.heller@daemons-point.com","sentAt":"2013-09-03T07:35:29Z","receivedAt":"2013-09-03T07:35:29Z","isPatch":true,"sender":{"key":"uli.heller@daemons-point.com","avatar":"https://avatars.githubusercontent.com/u/1677414?v=4"},"body":"When using git-svn in combination with serf-1.2.1 core dumps are\ncreated on termination. This is caused by a bug in serf, a fix for\nthe bug exists (see https://code.google.com/p/serf/source/detail?r=2146).\nNevertheless, I think it makes sense to fix the issue within the\ngit perl module Ra.pm, too. The change frees the private copy of\nthe remote access object on termination which prevents the error\nfrom happening.\n\nNote: Since subversion-1.8.0 and later do require serf-1.2.1 or later,\nthe core dumps typically do show up when upgrading to a recent version\nof subversion.\n\nCredits: Jonathan Lambrechts for proposing a fix to Ra.pm.\nEvgeny Kotkov and Ivan Zhakov for fixing the issue in serf and\npointing me to that fix.\n---\n perl/Git/SVN/Ra.pm | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\nindex 75ecc42..78dd346 100644\n--- a/perl/Git/SVN/Ra.pm\n+++ b/perl/Git/SVN/Ra.pm\n@@ -32,6 +32,11 @@ BEGIN {\n \t}\n }\n\n+END {\n+\t$RA = undef;\n+\t$ra_invalid = 1;\n+}\n+\n sub _auth_providers () {\n \tmy @rv = (\n \t  SVN::Client::get_simple_provider(),\n-- \n1.8.4\n"},{"id":"226869","messageId":"xmqqli3bhzop.fsf@gitster.dls.corp.google.com","threadId":"34838","inReplyTo":"6970c0cab40c60195c8f042a3b930a0a.squirrel@83.236.132.106","subject":"Re: [PATCH] git-svn: Fix termination issues for remote svn connections","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-05T18:48:54Z","receivedAt":"2013-09-05T18:48:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Uli Heller\" <uli.heller@daemons-point.com> writes:\n\n> When using git-svn in combination with serf-1.2.1 core dumps are\n> created on termination. This is caused by a bug in serf, a fix for\n> the bug exists (see https://code.google.com/p/serf/source/detail?r=2146).\n> Nevertheless, I think it makes sense to fix the issue within the\n> git perl module Ra.pm, too. The change frees the private copy of\n> the remote access object on termination which prevents the error\n> from happening.\n>\n> Note: Since subversion-1.8.0 and later do require serf-1.2.1 or later,\n> the core dumps typically do show up when upgrading to a recent version\n> of subversion.\n>\n> Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.\n> Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and\n> pointing me to that fix.\n> ---\n\nThanks.  Please sign-off your patch.\n\nI am Cc'ing Kyle McKay who apparently had some experience working\nwith git-svn with newer svn that can only use serf, hoping that we\ncan get an independent opinion/test just to be sure.  Also Cc'ed is\nEric Wong who has been the official git-svn area expert, but I\nunderstand that Eric hasn't needed to use git-svn for quite a while,\nso it is perfectly fine if he does not have any comment on this one.\n\nWe may want to find a volunteer to move \"git svn\" forward as a new\narea expert (aka subsystem maintainer), by the way.\n\n\n\n>  perl/Git/SVN/Ra.pm | 5 +++++\n>  1 file changed, 5 insertions(+)\n>\n> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\n> index 75ecc42..78dd346 100644\n> --- a/perl/Git/SVN/Ra.pm\n> +++ b/perl/Git/SVN/Ra.pm\n> @@ -32,6 +32,11 @@ BEGIN {\n>  \t}\n>  }\n>\n> +END {\n> +\t$RA = undef;\n> +\t$ra_invalid = 1;\n> +}\n> +\n>  sub _auth_providers () {\n>  \tmy @rv = (\n>  \t  SVN::Client::get_simple_provider(),\n"},{"id":"226871","messageId":"20130905190249.GA1532@dcvr.yhbt.net","threadId":"34838","inReplyTo":"xmqqli3bhzop.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git-svn: Fix termination issues for remote svn connections","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2013-09-05T19:02:49Z","receivedAt":"2013-09-05T19:02:49Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> \"Uli Heller\" <uli.heller@daemons-point.com> writes:\n> > Nevertheless, I think it makes sense to fix the issue within the\n> > git perl module Ra.pm, too. The change frees the private copy of\n> > the remote access object on termination which prevents the error\n> > from happening.\n\n> Thanks.  Please sign-off your patch.\n> \n> I am Cc'ing Kyle McKay who apparently had some experience working\n> with git-svn with newer svn that can only use serf, hoping that we\n> can get an independent opinion/test just to be sure.  Also Cc'ed is\n> Eric Wong who has been the official git-svn area expert, but I\n> understand that Eric hasn't needed to use git-svn for quite a while,\n> so it is perfectly fine if he does not have any comment on this one.\n> \n> We may want to find a volunteer to move \"git svn\" forward as a new\n> area expert (aka subsystem maintainer), by the way.\n\nCorrect, git-svn has the effect of being self-obsoleting.\n\nI agree with adding a workaround for broken things, however\nI suggest a code comment explaining why it is necessary.\nThe commit message is important, too, but might get harder to track\ndown if there's code movement/refactoring in the future.\n\n> > +END {\n> > +\t$RA = undef;\n> > +\t$ra_invalid = 1;\n> > +}\n"},{"id":"226892","messageId":"xmqqk3iug8sz.fsf@gitster.dls.corp.google.com","threadId":"34838","inReplyTo":"20130905190249.GA1532@dcvr.yhbt.net","subject":"Re: [PATCH] git-svn: Fix termination issues for remote svn connections","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-05T23:14:52Z","receivedAt":"2013-09-05T23:14:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <normalperson@yhbt.net> writes:\n\n> Junio C Hamano <gitster@pobox.com> wrote:\n>> \"Uli Heller\" <uli.heller@daemons-point.com> writes:\n>> > Nevertheless, I think it makes sense to fix the issue within the\n>> > git perl module Ra.pm, too. The change frees the private copy of\n>> > the remote access object on termination which prevents the error\n>> > from happening.\n>\n>> Thanks.  Please sign-off your patch.\n>> \n>> I am Cc'ing Kyle McKay who apparently had some experience working\n>> with git-svn with newer svn that can only use serf, hoping that we\n>> can get an independent opinion/test just to be sure.  Also Cc'ed is\n>> Eric Wong who has been the official git-svn area expert, but I\n>> understand that Eric hasn't needed to use git-svn for quite a while,\n>> so it is perfectly fine if he does not have any comment on this one.\n>> \n>> We may want to find a volunteer to move \"git svn\" forward as a new\n>> area expert (aka subsystem maintainer), by the way.\n>\n> Correct, git-svn has the effect of being self-obsoleting.\n>\n> I agree with adding a workaround for broken things, however\n> I suggest a code comment explaining why it is necessary.\n> The commit message is important, too, but might get harder to track\n> down if there's code movement/refactoring in the future.\n\nThanks for a good suggestion.  I agree that this addition is a good\nexample where in-code comment would really help the future readers.\n\n>\n>> > +END {\n>> > +\t$RA = undef;\n>> > +\t$ra_invalid = 1;\n>> > +}\n"},{"id":"226931","messageId":"A46AD76E-56FA-4555-8811-6141284300DD@gmail.com","threadId":"34838","inReplyTo":"xmqqli3bhzop.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git-svn: Fix termination issues for remote svn connections","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2013-09-06T11:46:30Z","receivedAt":"2013-09-06T11:46:30Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Sep 5, 2013, at 11:48, Junio C Hamano wrote:\n> \"Uli Heller\" <uli.heller@daemons-point.com> writes:\n>\n>> When using git-svn in combination with serf-1.2.1 core dumps are\n>> created on termination. This is caused by a bug in serf, a fix for\n>> the bug exists (see https://code.google.com/p/serf/source/detail?r=2146) \n>> .\n>> Nevertheless, I think it makes sense to fix the issue within the\n>> git perl module Ra.pm, too. The change frees the private copy of\n>> the remote access object on termination which prevents the error\n>> from happening.\n>>\n>> Note: Since subversion-1.8.0 and later do require serf-1.2.1 or  \n>> later,\n>> the core dumps typically do show up when upgrading to a recent  \n>> version\n>> of subversion.\n>>\n>> Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.\n>> Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and\n>> pointing me to that fix.\n>> ---\n>\n> Thanks.  Please sign-off your patch.\n>\n> I am Cc'ing Kyle McKay who apparently had some experience working\n> with git-svn with newer svn that can only use serf, hoping that we\n> can get an independent opinion/test just to be sure.  Also Cc'ed is\n> Eric Wong who has been the official git-svn area expert, but I\n> understand that Eric hasn't needed to use git-svn for quite a while,\n> so it is perfectly fine if he does not have any comment on this one.\n>\n> We may want to find a volunteer to move \"git svn\" forward as a new\n> area expert (aka subsystem maintainer), by the way.\n>\n>> perl/Git/SVN/Ra.pm | 5 +++++\n>> 1 file changed, 5 insertions(+)\n>>\n>> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\n>> index 75ecc42..78dd346 100644\n>> --- a/perl/Git/SVN/Ra.pm\n>> +++ b/perl/Git/SVN/Ra.pm\n>> @@ -32,6 +32,11 @@ BEGIN {\n>> \t}\n>> }\n>>\n>> +END {\n>> +\t$RA = undef;\n>> +\t$ra_invalid = 1;\n>> +}\n>> +\n>> sub _auth_providers () {\n>> \tmy @rv = (\n>> \t  SVN::Client::get_simple_provider(),\n\nI have not, as of yet, been able to reproduce the problem, so I cannot  \nverify the solution.  Maybe Uli can provide an example of a git-svn  \ncommand that demonstrates the failure?\n\nI am running a fresh build of subversion 1.8.3 with serf version 1.3.1  \n(the most recent serf release).\n\nAccording to the serf library history, version 1.3.1 of serf was  \ntagged at revision 2139 from revision 2138, but the serf fix mentioned  \nabove was checked in at revision 2146, so it cannot possibly be in the  \nserf 1.3.1 release.\n\nI'm using Git built from master (57e4c1783).  I see the same behavior  \nboth with and without the SVN/Ra.pm patch (and using both bulk updates  \nand skelta mode).  Does the problem not happen on a git svn clone?  I  \ncan force serf back to version 1.2.1 and try that version just to see,  \nbut I would like to have an example of a known failing git svn command  \nfor testing purposes.  Thanks.\n"},{"id":"226932","messageId":"07b9b270090d6b42515c5dc3dfb8ab4f.squirrel@83.236.132.106","threadId":"34838","inReplyTo":"A46AD76E-56FA-4555-8811-6141284300DD@gmail.com","subject":"Re: [PATCH] git-svn: Fix termination issues for remote svn connections","fromName":"Uli Heller","fromEmail":"uli.heller@daemons-point.com","sentAt":"2013-09-06T12:06:32Z","receivedAt":"2013-09-06T12:06:32Z","isPatch":true,"sender":{"key":"uli.heller@daemons-point.com","avatar":"https://avatars.githubusercontent.com/u/1677414?v=4"},"body":"On Fri, September 6, 2013 1:46 pm, Kyle J. McKay wrote:\n> On Sep 5, 2013, at 11:48, Junio C Hamano wrote:\n>> \"Uli Heller\" <uli.heller@daemons-point.com> writes:\n>>\n>>> When using git-svn in combination with serf-1.2.1 core dumps are\n>>> created on termination. This is caused by a bug in serf, a fix for\n>>> the bug exists (see\n>>> https://code.google.com/p/serf/source/detail?r=2146)\n>>> .\n>>> Nevertheless, I think it makes sense to fix the issue within the\n>>> git perl module Ra.pm, too. The change frees the private copy of\n>>> the remote access object on termination which prevents the error\n>>> from happening.\n>>>\n>>> Note: Since subversion-1.8.0 and later do require serf-1.2.1 or\n>>> later,\n>>> the core dumps typically do show up when upgrading to a recent\n>>> version\n>>> of subversion.\n>>>\n>>> Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.\n>>> Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and\n>>> pointing me to that fix.\n>>> ---\n>>\n>> Thanks.  Please sign-off your patch.\n>>\n>> I am Cc'ing Kyle McKay who apparently had some experience working\n>> with git-svn with newer svn that can only use serf, hoping that we\n>> can get an independent opinion/test just to be sure.  Also Cc'ed is\n>> Eric Wong who has been the official git-svn area expert, but I\n>> understand that Eric hasn't needed to use git-svn for quite a while,\n>> so it is perfectly fine if he does not have any comment on this one.\n>>\n>> We may want to find a volunteer to move \"git svn\" forward as a new\n>> area expert (aka subsystem maintainer), by the way.\n>>\n>>> perl/Git/SVN/Ra.pm | 5 +++++\n>>> 1 file changed, 5 insertions(+)\n>>>\n>>> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\n>>> index 75ecc42..78dd346 100644\n>>> --- a/perl/Git/SVN/Ra.pm\n>>> +++ b/perl/Git/SVN/Ra.pm\n>>> @@ -32,6 +32,11 @@ BEGIN {\n>>> \t}\n>>> }\n>>>\n>>> +END {\n>>> +\t$RA = undef;\n>>> +\t$ra_invalid = 1;\n>>> +}\n>>> +\n>>> sub _auth_providers () {\n>>> \tmy @rv = (\n>>> \t  SVN::Client::get_simple_provider(),\n>\n> I have not, as of yet, been able to reproduce the problem, so I cannot\n> verify the solution.  Maybe Uli can provide an example of a git-svn\n> command that demonstrates the failure?\n>\n> I am running a fresh build of subversion 1.8.3 with serf version 1.3.1\n> (the most recent serf release).\n>\n> According to the serf library history, version 1.3.1 of serf was\n> tagged at revision 2139 from revision 2138, but the serf fix mentioned\n> above was checked in at revision 2146, so it cannot possibly be in the\n> serf 1.3.1 release.\n>\n> I'm using Git built from master (57e4c1783).  I see the same behavior\n> both with and without the SVN/Ra.pm patch (and using both bulk updates\n> and skelta mode).  Does the problem not happen on a git svn clone?  I\n> can force serf back to version 1.2.1 and try that version just to see,\n> but I would like to have an example of a known failing git svn command\n> for testing purposes.  Thanks.\n\nI think this command should produce the error:\n\n  git svn clone --stdlayout https://github.com/uli-heller/uli-javawrapper\n\nYou can use any other svn repo as well, you only have to specify an HTTPS\nurl.\n\n[Yes, I know you typically don't clone github via git svn]\n\nI'll do some more tests using git HEAD and serf 1.3.1 when I'm back home.\n\nThanks + best regards, Uli\n"},{"id":"226933","messageId":"9D2EC674-887A-479B-B8FB-3EFEAC391435@gmail.com","threadId":"34838","inReplyTo":"07b9b270090d6b42515c5dc3dfb8ab4f.squirrel@83.236.132.106","subject":"Re: [PATCH] git-svn: Fix termination issues for remote svn connections","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2013-09-06T12:44:07Z","receivedAt":"2013-09-06T12:44:07Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Sep 6, 2013, at 05:06, Uli Heller wrote:\n>> I'm using Git built from master (57e4c1783).  I see the same behavior\n>> both with and without the SVN/Ra.pm patch (and using both bulk  \n>> updates\n>> and skelta mode).  Does the problem not happen on a git svn clone?  I\n>> can force serf back to version 1.2.1 and try that version just to  \n>> see,\n>> but I would like to have an example of a known failing git svn  \n>> command\n>> for testing purposes.  Thanks.\n>\n> I think this command should produce the error:\n>\n>  git svn clone --stdlayout https://github.com/uli-heller/uli-javawrapper\n>\n> You can use any other svn repo as well, you only have to specify an  \n> HTTPS\n> url.\n\nYes, that does it.  Interesting that cloning from \"https://github.com/uli-heller/uli-javawrapper \n\" core dumps while cloning from \"http://github.com/uli-heller/uli-javawrapper \n\" does not even though the latter redirects to \"https://github.com/uli-heller/uli-javawrapper \n\".\n\nIn any case, I can now reproduce the problem (serf 1.3.1 still breaks  \nsince it does not yet contain the fix and it is the most recent serf  \nrelease available).\n\nAnd the Git/SVN/Ra.pm fix does eliminate the problem for me (both with  \nbulk updates and with skelta updates -- the crash occurs with either).\n"},{"id":"226934","messageId":"E8831080-6FD3-4276-BE80-F0E9BAC65AA3@gmail.com","threadId":"34838","inReplyTo":"xmqqli3bhzop.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] git-svn: Fix termination issues for remote svn connections","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2013-09-06T12:44:20Z","receivedAt":"2013-09-06T12:44:20Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Sep 5, 2013, at 11:48, Junio C Hamano wrote:\n> I am Cc'ing Kyle McKay who apparently had some experience working\n> with git-svn with newer svn that can only use serf, hoping that we\n> can get an independent opinion/test just to be sure.\n\nOn Sep 3, 2013, at 00:35, Uli Heller wrote:\n> When using git-svn in combination with serf-1.2.1 core dumps are\n> created on termination. This is caused by a bug in serf, a fix for\n> the bug exists (see https://code.google.com/p/serf/source/detail?r=2146) \n> .\n> Nevertheless, I think it makes sense to fix the issue within the\n> git perl module Ra.pm, too. The change frees the private copy of\n> the remote access object on termination which prevents the error\n> from happening.\n>\n> Note: Since subversion-1.8.0 and later do require serf-1.2.1 or later,\n> the core dumps typically do show up when upgrading to a recent version\n> of subversion.\n>\n> Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.\n> Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and\n> pointing me to that fix.\n> ---\n\nIn addition to the needed 'Signed-off-by:' a mention should be made  \nhere of the need to use 'https:' to reproduce the problem.\n\n> perl/Git/SVN/Ra.pm | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\n> index 75ecc42..78dd346 100644\n> --- a/perl/Git/SVN/Ra.pm\n> +++ b/perl/Git/SVN/Ra.pm\n> @@ -32,6 +32,11 @@ BEGIN {\n> \t}\n> }\n>\n> +END {\n> +\t$RA = undef;\n> +\t$ra_invalid = 1;\n> +}\n> +\n> sub _auth_providers () {\n> \tmy @rv = (\n> \t  SVN::Client::get_simple_provider(),\n> -- \n> 1.8.4\n\nTested-by: Kyle J. McKay <mackyle@gmail.com>\n\nI was able to reproduce the core dumps using subversion 1.8.3 (the  \nmost recent subversion release) with serf 1.3.1 (the most recent serf  \nrelease) and verify that this patch eliminates the core dump at git- \nsvn termination time.\n"},{"id":"226936","messageId":"f2fe486a6ca0bf40be4489bfe888d517.squirrel@83.236.132.106","threadId":"34838","inReplyTo":"9D2EC674-887A-479B-B8FB-3EFEAC391435@gmail.com","subject":"Re: [PATCH] git-svn: Fix termination issues for remote svn connections","fromName":"Uli Heller","fromEmail":"uli.heller@daemons-point.com","sentAt":"2013-09-06T13:18:32Z","receivedAt":"2013-09-06T13:18:32Z","isPatch":true,"sender":{"key":"uli.heller@daemons-point.com","avatar":"https://avatars.githubusercontent.com/u/1677414?v=4"},"body":"On Fri, September 6, 2013 2:44 pm, Kyle J. McKay wrote:\n> On Sep 6, 2013, at 05:06, Uli Heller wrote:\n>>> I'm using Git built from master (57e4c1783).  I see the same behavior\n>>> both with and without the SVN/Ra.pm patch (and using both bulk\n>>> updates\n>>> and skelta mode).  Does the problem not happen on a git svn clone?  I\n>>> can force serf back to version 1.2.1 and try that version just to\n>>> see,\n>>> but I would like to have an example of a known failing git svn\n>>> command\n>>> for testing purposes.  Thanks.\n>>\n>> I think this command should produce the error:\n>>\n>>  git svn clone --stdlayout https://github.com/uli-heller/uli-javawrapper\n>>\n>> You can use any other svn repo as well, you only have to specify an\n>> HTTPS\n>> url.\n>\n> Yes, that does it.  Interesting that cloning from\n> \"https://github.com/uli-heller/uli-javawrapper\n> \" core dumps while cloning from\n> \"http://github.com/uli-heller/uli-javawrapper\n> \" does not even though the latter redirects to\n> \"https://github.com/uli-heller/uli-javawrapper\n> \".\n>\n> In any case, I can now reproduce the problem (serf 1.3.1 still breaks\n> since it does not yet contain the fix and it is the most recent serf\n> release available).\n>\n> And the Git/SVN/Ra.pm fix does eliminate the problem for me (both with\n> bulk updates and with skelta updates -- the crash occurs with either).\n\nGreat. So I don't have to run any more tests ;)\n\nWhat shall I do next? Add some inline comments to the patch?\n\n---\nBest regards, Uli.\n"},{"id":"226950","messageId":"xmqqa9jpzyvo.fsf@gitster.dls.corp.google.com","threadId":"34838","inReplyTo":"f2fe486a6ca0bf40be4489bfe888d517.squirrel@83.236.132.106","subject":"Re: [PATCH] git-svn: Fix termination issues for remote svn connections","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-06T16:41:15Z","receivedAt":"2013-09-06T16:41:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Uli Heller\" <uli.heller@daemons-point.com> writes:\n\n> On Fri, September 6, 2013 2:44 pm, Kyle J. McKay wrote:\n> ...\n>> In any case, I can now reproduce the problem (serf 1.3.1 still breaks\n>> since it does not yet contain the fix and it is the most recent serf\n>> release available).\n>>\n>> And the Git/SVN/Ra.pm fix does eliminate the problem for me (both with\n>> bulk updates and with skelta updates -- the crash occurs with either).\n>\n> Great. So I don't have to run any more tests ;)\n>\n> What shall I do next? Add some inline comments to the patch?\n\nSomething like this?\n\n-- >8 --\n\nFrom: Uli Heller <uli.heller@daemons-point.com>\nSubject: [PATCH] git-svn: fix termination issues for remote svn connections\n\ngit-svn used in combination with serf to talk to svn repository\nserved over HTTPS dumps core on termination.\n\nThis is caused by a bug in serf, and the most recent serf release\n1.3.1 still exhibits the problem; a fix for the bug exists (see\nhttps://code.google.com/p/serf/source/detail?r=2146).\n\nUntil the bug is fixed, work around the issue within the git perl\nmodule Ra.pm by freeing the private copy of the remote access object\non termination, which seems to be sufficient to prevent the error\nfrom happening.\n\nNote: Since subversion-1.8.0 and later do require serf-1.2.1 or\nlater, this issue typically shows up when upgrading to a recent\nversion of subversion.\n\nCredits go to Jonathan Lambrechts for proposing a fix to Ra.pm,\nEvgeny Kotkov and Ivan Zhakov for fixing the issue in serf and\npointing me to that fix.\n\nSigned-off-by: Uli Heller <uli.heller@daemons-point.com>\nTested-by: Kyle J. McKay <mackyle@gmail.com>\n---\n perl/Git/SVN/Ra.pm | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\nindex 75ecc42..78dd346 100644\n--- a/perl/Git/SVN/Ra.pm\n+++ b/perl/Git/SVN/Ra.pm\n@@ -32,6 +32,14 @@ BEGIN {\n \t}\n }\n \n+# serf has a bug that leads to a coredump upon termination if the\n+# remote access object is left around (not fixed yet in serf 1.3.1).\n+# Explicitly free it to work around the issue.\n+END {\n+\t$RA = undef;\n+\t$ra_invalid = 1;\n+}\n+\n sub _auth_providers () {\n \tmy @rv = (\n \t  SVN::Client::get_simple_provider(),\n-- \n1.8.4-431-g94f3a6b\n"},{"id":"227200","messageId":"522D643E.2000901@daemons-point.com","threadId":"34838","inReplyTo":"xmqqa9jpzyvo.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2] git-svn: Fix termination issues for remote svn connections","fromName":"Uli Heller","fromEmail":"uli.heller@daemons-point.com","sentAt":"2013-09-09T06:01:34Z","receivedAt":"2013-09-09T06:01:34Z","isPatch":true,"sender":{"key":"uli.heller@daemons-point.com","avatar":"https://avatars.githubusercontent.com/u/1677414?v=4"},"body":" From a763550fc59b756580f9b162839d2737c27f2fe3 Mon Sep 17 00:00:00 2001\nFrom: Uli Heller <uli.heller@daemons-point.com>\nDate: Tue, 3 Sep 2013 09:14:03 +0200\nSubject: [PATCH] git-svn: Fix termination issues for remote svn connections\n\nWhen using git-svn in combination with serf-1.2.1 core dumps are\ncreated on termination. This is caused by a bug in serf, a fix for\nthe bug exists (see https://code.google.com/p/serf/source/detail?r=2146).\nNevertheless, I think it makes sense to fix the issue within the\ngit perl module Ra.pm, too. The change frees the private copy of\nthe remote access object on termination which prevents the error\nfrom happening.\n\nNote: Since subversion-1.8.0 and later do require serf-1.2.1 or later,\nthe core dumps typically do show up when upgrading to a recent version\nof subversion.\n\nCredits: Jonathan Lambrechts for proposing a fix to Ra.pm.\nEvgeny Kotkov and Ivan Zhakov for fixing the issue in serf and\npointing me to that fix.\nJunio C Hamano for proposing a great code comment.\nKyle J. McKay for testing against latest serf (1.3.1).\n... and various others - thanks!\n---\n  perl/Git/SVN/Ra.pm | 8 ++++++++\n  1 file changed, 8 insertions(+)\n\ndiff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\nindex 75ecc42..a7b0119 100644\n--- a/perl/Git/SVN/Ra.pm\n+++ b/perl/Git/SVN/Ra.pm\n@@ -32,6 +32,14 @@ BEGIN {\n         }\n  }\n\n+# serf has a bug that leads to a coredump upon termination if the\n+# remote access object is left around (not fixed yet in serf 1.3.1).\n+# Explicitly free it to work around the issue.\n+END {\n+       $RA = undef;\n+       $ra_invalid = 1;\n+}\n+\n  sub _auth_providers () {\n         my @rv = (\n           SVN::Client::get_simple_provider(),\n-- \n1.8.4\n"},{"id":"227281","messageId":"xmqqfvtd1u7n.fsf@gitster.dls.corp.google.com","threadId":"34838","inReplyTo":"522D643E.2000901@daemons-point.com","subject":"Re: [PATCH v2] git-svn: Fix termination issues for remote svn connections","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-09T15:42:10Z","receivedAt":"2013-09-09T15:42:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Uli Heller <uli.heller@daemons-point.com> writes:\n\n> From a763550fc59b756580f9b162839d2737c27f2fe3 Mon Sep 17 00:00:00 2001\n> From: Uli Heller <uli.heller@daemons-point.com>\n> Date: Tue, 3 Sep 2013 09:14:03 +0200\n> Subject: [PATCH] git-svn: Fix termination issues for remote svn connections\n\nNone of the above need to be in the e-mail body (the first one must\nnot be in the e-mail body, and the other three are redundant).\n\n> When using git-svn in combination with serf-1.2.1 core dumps are\n> created on termination. This is caused by a bug in serf, a fix for\n> the bug exists (see https://code.google.com/p/serf/source/detail?r=2146).\n> Nevertheless, I think it makes sense to fix the issue within the\n> git perl module Ra.pm, too. The change frees the private copy of\n> the remote access object on termination which prevents the error\n> from happening.\n>\n> Note: Since subversion-1.8.0 and later do require serf-1.2.1 or later,\n> the core dumps typically do show up when upgrading to a recent version\n> of subversion.\n>\n> Credits: Jonathan Lambrechts for proposing a fix to Ra.pm.\n> Evgeny Kotkov and Ivan Zhakov for fixing the issue in serf and\n> pointing me to that fix.\n> Junio C Hamano for proposing a great code comment.\n> Kyle J. McKay for testing against latest serf (1.3.1).\n> ... and various others - thanks!\n> ---\n\nHmph, the only change I see relative to the version in the message\nyou are responding to is reversion of the proposed commit log\nmessage rewrite I did, and missing sign-off from you, and the latter\nof which matters most.\n\nI'd keep the one queued on 'pu' 73ffac3b (git-svn: fix termination\nissues for remote svn connections, 2013-09-03) for now.\n\nThanks.\n\n>  perl/Git/SVN/Ra.pm | 8 ++++++++\n>  1 file changed, 8 insertions(+)\n>\n> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\n> index 75ecc42..a7b0119 100644\n> --- a/perl/Git/SVN/Ra.pm\n> +++ b/perl/Git/SVN/Ra.pm\n> @@ -32,6 +32,14 @@ BEGIN {\n>         }\n>  }\n>\n> +# serf has a bug that leads to a coredump upon termination if the\n> +# remote access object is left around (not fixed yet in serf 1.3.1).\n> +# Explicitly free it to work around the issue.\n> +END {\n> +       $RA = undef;\n> +       $ra_invalid = 1;\n> +}\n> +\n>  sub _auth_providers () {\n>         my @rv = (\n>           SVN::Client::get_simple_provider(),\n"}]}